Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WeakMaps/WeakSets need not include values which are not referenced elsewhere in output #225

Open
overlookmotel opened this issue Jul 14, 2021 · 1 comment
Labels
optimization Improvement to performance or output

Comments

@overlookmotel
Copy link
Owner

WeakMaps and WeakSets should not include elements which are not referenced elsewhere in the exported app.

Currently new WeakSet( [ {x: 1} ] ) is serialized as new WeakSet( [ {x: 1} ] ).

This is pointless as the object {x: 1} is not exposed anywhere else in the exported app, and therefore it's never possible for the app to test whether the object is in the WeakSet or not (weakSet.has( obj ) requires the object to start with).

Cannot rely on garbage collector to do this automatically. The object might be referenced elsewhere in the code being run at time of serialization and therefore not available for garbage collection, but not be part of the value being serialized.

Will be tricky to implement for WeakMaps. e.g. a case like this:

  • 2 WeakMaps weakMap1 and weakMap2.
  • Object obj1 is referenced in the serialized output.
  • obj1 is used as a key in weakMap1, mapping to object obj2.
  • obj2 is used as a key in weakMap2 mapping to object obj3.

In this case determining whether obj3 needs to be serialized or not depends on whether obj1 is referenced elsewhere other than in weakMap1.

Probably need to leave serializing WeakMaps until last. Values for keys which are referenced elsewhere are serialized and inserted into their WeakMaps. That may make other values valid keys that need to be added to other WeakMaps, so need to repeat the check again and again until all possible avenues are exhausted.

The order of elements in WeakMaps/WeakSets is not relevant - there's no way to iterate them - so that simplifies things a bit.

@overlookmotel overlookmotel added the optimization Improvement to performance or output label Jul 14, 2021
@overlookmotel overlookmotel changed the title WeakMaps/WeakSets should not include values which are not referenced elsewhere in import WeakMaps/WeakSets need not include values which are not referenced elsewhere in export Jul 14, 2021
@overlookmotel overlookmotel changed the title WeakMaps/WeakSets need not include values which are not referenced elsewhere in export WeakMaps/WeakSets need not include values which are not referenced elsewhere in output Sep 30, 2021
@overlookmotel
Copy link
Owner Author

This would be quite simple if two-phase serialization (#426) were implemented.

WeakSets

  • Initialize var at start of tracing weakSets as empty array.
  • When a new WeakSet is encountered, create a record for it (with an empty members array) and add to weakSets.
  • At end of tracing, for each record in weakSets, iterate over members of the WeakSet. Any members which have a record for them are added to record.members and output as member of the WeakSet.

Actually this would not require #426 to be implemented first. Could be implemented in current code base.

WeakMaps

  • All records contain a property .keyForWeakMaps set as undefined initially.
  • When a WeakMap is encountered for first time, create a record for it.
  • Record has a members property which is initially an empty array.
  • For each key of the WeakMap:
    • If key already has a corresponding record, trace the value, and add records for key and value to record.members as a pair.
    • Otherwise, create a record for key, but don't trace the key's value or the weakly-referenced value. Set .keyForWeakMaps property on key's record to [ weakMapRecord ].
  • When any other value is encountered, if it has an existing record with .keyForWeakMaps defined:
    • Make a copy of record.keyForWeakMaps.
    • Set record.keyForWeakMaps = undefined.
    • Trace the key's value.
    • For each WeakMap record in keyForWeakMaps, look up the value for the key, and trace the value.
    • Add records for key and value to weakMapRecord.members as a pair.

At end of tracing, only strongly-referenced key-value pairs will be recorded as members of the WeakMaps, and only they will appear in output.

This requires two-phase serialization, because at the time a WeakMap is encountered, what members it should include is unknown. This can only be known at the end of tracing of the whole program. It's not possible to revisit WeakMaps late the way Livepack is currently written, because it wouldn't be possible to identify circular references at that stage.

NB WeakSets and WeakMaps are not iterable, so the order of their members is not relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimization Improvement to performance or output
Projects
None yet
Development

No branches or pull requests

1 participant