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

Pre-init all sub-encoders of the builder encoder #1613

Merged
merged 2 commits into from
Feb 26, 2025

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Feb 26, 2025

  • Use WeakReference to prevent circular reference
  • Avoid having the same encoder initialized multiple times for all the stage and operator classes

The overhead of instantiating all these objects in the constructor is offset by the fact that we don't have to check each call.

image

Use weak reference to prevent circular reference

Avoid having the same encoder initialized multiple times
@GromNaN GromNaN requested a review from a team as a code owner February 26, 2025 12:57
@GromNaN GromNaN requested review from jmikola and alcaeus February 26, 2025 12:57
trait RecursiveEncode
{
final public function __construct(protected readonly BuilderEncoder $encoder)
/** @param WeakReference<Encoder> $encoder */
final public function __construct(private readonly WeakReference $encoder)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to enforce the type of the class the WeakReference is pointing to? Would that entail checking the type of $encoder->get() and manually throwing a TypeError if it's wrong?

I don't think we need to do this here since this is an internal trait, mainly just asking out of curiosity.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You would like native support for generics, that doesn't exist.
Also, the WeakReference class is final, we cannot extend it to restrict the type.

@GromNaN GromNaN enabled auto-merge (squash) February 26, 2025 14:30
@GromNaN GromNaN merged commit 481cffa into mongodb:v1.x Feb 26, 2025
31 checks passed
@GromNaN GromNaN deleted the weak-ref branch February 26, 2025 15:33
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.

2 participants