Reduce the size of type information in durable coroutine state #114
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chriso
changed the title
achille-roussel
approved these changes
Nov 21, 2023
In all cases, IDs start at one; zero is used as the nil sentinel, so there's no need for signed ints.
Although these are likely unique, interning them with all other strings means that get more accurate stats when looking at the size of different parts of the durable coroutine state.
chriso
added a commit
that referenced
this pull request
Nov 21, 2023
This fixes bugs introduced by 24b7af8. Since #114, we no longer store type information for types with custom serializers registered. We allow custom serializers to be registered for interface types, and since #104 these custom routines are used for all implementations of those interfaces. We need to store enough information to be able to recreate the original type in the program that generated the state. Since no stable identifier is available for types, we currently use the offset to a known anchor in the program (see [1](https://github.com/stealthrocket/coroutine/blob/00b9eb66a6472279b8dc9a4994b2694ff3f06c17/types/unsafe.go#L66-L84) and [2](https://github.com/stealthrocket/coroutine/blob/00b9eb66a6472279b8dc9a4994b2694ff3f06c17/types/types.go#L57-L59)). The first bug introduced by #114 is that it wasn't continuing to use this facility when a custom serializer had been registered, so the serialization layer was erroneously returning the interface type when deserializing all implementations of those interface types. The second bug was more subtle; when `*A` implements the interface `T` but `A` is the named type for which an offset is available, we need to retain this information so that we can correctly create `*A` rather than `T` when deserializing objects.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now that we can inspect durable coroutine state (#113), a few optimizations are clear.
Firstly, package names are currently duplicated across types. This PR updates the serialization layer to intern strings so that there's no duplication.
Secondly, the serialization layer currently stores type information for all referenced types (recursively). When custom serializers are registered this type information is not used; we just need a reference to the custom serialization routines. An offline process analyzing the state doesn't need this information either, because the custom serializer might emit entirely different objects/types. Since #112, type information is available for the output of custom serializers, so the input type info isn't necessary. This PR updates the serialization layer to no longer store type information for types with custom serializers. All that's stored now is the interned package name and type name, along with the
customflag which indicate that the type is opaque. When stacked with #104, the reduction in the size of type information can be drastic.