-
-
Notifications
You must be signed in to change notification settings - Fork 471
feat: implement proxies support #873
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
base: master
Are you sure you want to change the base?
Conversation
6a7e5d6 to
c1e84a1
Compare
| } | ||
|
|
||
| func Fetch(from, i any) any { | ||
| if proxy, ok := from.(Proxy); ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Relying only on real time to check for proxy interface is not the best strategy. We need to do a pre-optimization and check at compile time whether a special opcode for fetching using this interface can be inserted ahead of time. With just a runtime check for proxy, I cannot accept this pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've a couple of questions:
- By ahead of time optimization, do you mean to implement env check in
optimizer.Optimizemethod?- Current API allows using different envs for parsing, optimization and evaluation.
This means that during eval stage, user can supply an env that may have a proxy (e.g.foo.bar.baz), but this wasn't accounted during parse/optimize stage, so iface check still might be required insideOpFetch. - Proxy may be located deeply inside env (or populated at runtime), or returned from a func.
- Initially, I wanted to use type check instead of an interface as it is 2x cheaper, but couldn't find a suitable approach (see benchmark). Happy to accept different proposals.
- As 90% of users probably won't need proxies, this feature can be opt-in (feature flag). This will keep perf the same as before and will skip proxy checks by default.
- Current API allows using different envs for parsing, optimization and evaluation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By ahead of time optimization, do you mean to implement env check in optimizer.Optimize method?
The compiler. Compiler can produce optimized code.
Proxy may be located deeply inside env (or populated at runtime), or returned from a func.
Yes, in this case we can fallback to a common fetcher.
type check instead of an interface as it is 2x cheaper,
Even better to not do this at runtime at all!
As 90% of users probably won't need proxies, this feature can be opt-in (feature flag). This will keep perf the same as before and will skip proxy checks by default.
Yes, most of user will not need it. Idealy we hide this behide compier and do not expose additional checks at Fetch.
This PR adds support of proxies. Proxies allow intercepting property access, like Python's
__get__magic method.Proxies allow implementing different patterns like prototype chaining, read on demand, etc.
Fixes: #777