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

Simplify the API between factories and construction contexts by moving more logic into InternalContext. #1854

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

copybara-service[bot]
Copy link

Simplify the API between factories and construction contexts by moving more logic into InternalContext.

Instead of having factories manage a ConstructionContext object and call some subset of methods on it, we simplify things into a number of abstract operations on InternalContext

  • tryStartConstruction: called when we are starting construction
    • This is amortized O(1) work and only allocates when we are resizing the internal hash tables.
  • finishConstruction: called when construction is complete, clears the table
    • This is amortized O(1) work due to the hash search and some datastructure maintenance
  • finishConstructionAndSetReference: called by ConstructorInjector to support member injectors calling back into it
  • clearCurrentReference: called by ConstructorInjector after member injection is complete.

This enables the following optimizations

  • When disableCircularProxies is set can get away with a single int[] to track the set of currently constructing factories and turn finishConstructionAndSetReference into a no-op
  • When circular proxies are enabled we can eliminate the ConstructionContext object and only allocate when we actually construct a proxy.
  • We can 'statically' allocate each factory a unique non-zero integer (a circularFactoryId). This allows us to create a open-addressed hashtable with integer keys that reduces allocation, hashing and GC overhead.
    • this does limit us to 2^32-1 circular factories before we get collisions, but that should be enough! pretty sure that other things will fail first.
  • The new InternalContext protocol enables some simplifications in the factories which allows to delete an implementation

Previously we would insert ConstructionContext objects into the hash table and reuse them if the same factory was used multiple times. This was generally rare, and now we instead remove keys from the table after construction. In theory this means we always perform 2 hash table lookups per construction where the prior approach might only do one. This is true but in general all lookups missed so the prior approach also required 2 lookups (get and put).

This is an optimization all on its own, but the new simpler protocol will enable more optimizations in a followup.

@copybara-service copybara-service bot force-pushed the test_680304599 branch 2 times, most recently from c1c50ca to 0ea1900 Compare January 9, 2025 21:32
…g more logic into InternalContext.

Instead of having factories manage a `ConstructionContext` object and call some subset of methods on it, we simplify things into a number of abstract operations on `InternalContext`
 - `tryStartConstruction`: called when we are starting construction
      - This is amortized O(1) work and only allocates when we are resizing the internal hash tables.
 - `finishConstruction`: called when construction is complete, clears the table
      - This is amortized O(1) work due to the hash search and some datastructure maintenance
 - `finishConstructionAndSetReference`: called by `ConstructorInjector` to support member injectors calling back into it
 - `clearCurrentReference`: called by `ConstructorInjector` after member injection is complete.

This enables the following optimizations

* When `disableCircularProxies` is set can get away with a single `int[]` to track the set of currently constructing factories and turn `finishConstructionAndSetReference` into a no-op
* When circular proxies are enabled we can eliminate the `ConstructionContext` object and only allocate when we actually construct a proxy.
* We can 'statically' allocate each factory a unique non-zero integer (a `circularFactoryId`). This allows us to create a open-addressed hashtable with integer keys that reduces allocation, hashing and GC overhead.
  - this does limit us to 2^32-1 circular factories before we get collisions, but that should be enough! pretty sure that other things will fail first.
* The new `InternalContext` protocol enables some simplifications in the factories which allows to delete an implementation

Previously we would insert `ConstructionContext` objects into the hash table and reuse them if the same factory was used multiple times.  This was generally rare, and now we instead remove keys from the table after construction.  In theory this means we always perform 2 hash table lookups per construction where the prior approach might only do one.  This is true but in general all lookups missed so the prior approach also required 2 lookups (`get` and `put`).

This is an optimization all on its own, but the new simpler protocol will enable more optimizations in a followup.

PiperOrigin-RevId: 713781971
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant