Investigate lifting 16-bit class-id limit in Dart VM

[mkustermann]

The first customers have started to hit 16-bit class id limit in JIT mode. See b/160229360

We should investigate lifting our 16-bit class-id restriction and see what the code size and heap usage impact is.

We could possibly also investigate lifting the two word heap alignment requirement, thereby freeing up space in heap objects (on average). Possibly also shrinking identity hash code (or handling it differently).

[rmacnak-google]

Many years ago, the class id field was 16-bits on both 32- and 64-bit systems, and the extra 32-bits in the 64-bit header was wasted. Then I increased the class id and size fields on 64-bit systems (and added classid_t). Then Erik reduced the class id field back to 16-bits and used the extra 32-bits to store the identity hash.

On 64-bit systems, we could swap the sizes of the class id and identity hash fields if a 16-bit hash is acceptable.

[mraleph]

Why do we assign class ids eagerly when we read classes? We should be able to assign them lazily when class is finalised for instantiation.

This does introduce certain amount of non-determinism and makes JITed code slower (potentially) due to more random range assignments, but it should probably considerably cut the size of the class table.

[mkustermann]

Why do we assign class ids eagerly when we read classes?

IIRC One of the reasons is that isolate communication via snapshots embeds class ids in the messages. So if classes are assigned class ids lazily, two isolates can have different numbering and bad things will happen ...

Once we are done with isolate groups, we could make separate isolate groups only communicate via json-like types then we could remove this.

In general I think we just have to face reality that with the increasing success of flutter and dart, we'll have to run bigger and bigger apps and our 16-bit limitation might need to be lifted, sooner or later.

[a-siva]

Many years ago, the class id field was 16-bits on both 32- and 64-bit systems, and the extra 32-bits in the 64-bit header was wasted. Then I increased the class id and size fields on 64-bit systems (and added classid_t). Then Erik reduced the class id field back to 16-bits and used the extra 32-bits to store the identity hash.

On 64-bit systems, we could swap the sizes of the class id and identity hash fields if a 16-bit hash is acceptable.

How do we determine if a 16-bit hash is acceptable?

[mkustermann]

How do we determine if a 16-bit hash is acceptable?

My impression is that it might not be acceptable - in particular for 64-bit systems (which is what this discussion is about), since we often have big heaps in 64-bit systems. Some of our own tools make heavy use of identity hash codes, where 16 bits might be a huge issue.

I was assigned on b/160229360 and started making some prototypes.

[mkustermann]

I've looked into not assigning any ids to the toplevel classes: Right now it seems that toplevel class ids are both used in the service protocol as well as in types (in connection with mirrors).

To unblock the customer, I think it's best to land cl/153607 first. I'll still spend a little more time investigating whether the issues mentioned above could be fixed. If so, we can revert cl/153607 and land an alternative solution.

[mkustermann]

I'll still spend a little more time investigating whether the issues mentioned above could be fixed.

Made cl/156003 which shows the changes we would need to make for not assigning class ids to top-level classes (Ryan's idea).

Though the changes in that CL are not sufficient yet, there are still remaining places where we use the top-level class ids in type objects and expose those type object to Dart code, e.g. in NoSuchMethod handling, see e.g. object.cc:

    if (throw_nsm_if_absent) {
      return ThrowNoSuchMethod(
          AbstractType::Handle(Class::Handle(toplevel_class()).RareType()),
          getter_name, Object::null_array(), Object::null_array(),
          InvocationMirror::kTopLevel, InvocationMirror::kGetter);
    }

which calls NoSuchMethodError._throwNew() with the type object of a top-level class as receiver, see error_patch.dart:

@patch
class NoSuchMethodError {
  final Object? _receiver;
  final Invocation _invocation;

  @pragma("vm:entry-point", "call")
  static void _throwNew(Object receiver /* <= Type { type_class_id: top-level-cid } */, ...) {
    throw new NoSuchMethodError._withType(receiver, memberName, invocationType,
        typeArgumentsLength, typeArguments, arguments, argumentNames);
  }

As cl/156003 shows we would need to special-case top-level classes in various places. Not convinced that it's better than the current solution of assigning class ids to top-level classes but outside the 16-bit range. Will therefore stop the investigation of this approach here.

[kf6gpe]

cc @kf6gpe