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

avm2: Implement ScopeChain caching #9378

Merged
merged 1 commit into from
Feb 9, 2023
Merged

Conversation

Bale001
Copy link
Member

@Bale001 Bale001 commented Feb 1, 2023

Implements the ability for ScopeChain's to cache their results for better performance. Currently to avoid any problems with dynamic properties, caching is disabled for any ScopeChains that contain a with scope.

@Bale001
Copy link
Member Author

Bale001 commented Feb 1, 2023

BTW: If we ever implement an optimizer such as the one in #7920, we can actually do something even better than this since it would then be possible to resolve everything before the function is even called.

@adrian17 adrian17 self-requested a review February 1, 2023 22:43
@adrian17
Copy link
Collaborator

adrian17 commented Feb 1, 2023

BTW: If we ever implement an optimizer such as the one in #7920, we can actually do something even better than this since it would then be possible to resolve everything before the function is even called.

I'm not 100% sure that's true.
This is related what we talked about in the past - a lot of lookups we do (op_coerce/op_astype/op_istype or resolve_type) shouldn't use the scope chain at all - in fact, it's incorrect to do so and it may be possible to construct some evil code that breaks current Ruffle due to this. Types (and more?) should instead use some global (not sure if fully-global or abc-global or what) registry that's not recursive at all.
(this would also let us remove the resolve_class_private weirdness)

The main(?) difference from optimization POV is that most/all the lookups using this global registry can be fully resolved eagerly - like, you can resolve function arg types and store them forever. And this is not really an optimization, things totally rely on types not changing at runtime. However the "real" getlex/findproperty lookups might be able to optimize to getting a specific scope with getglobal/outerscope (not sure if even that it possible 100% of time), but you still need to getproperty/getslot the actual value at runtime, as the value can be modified at runtime.

EDIT: I made an "evil swf" where the Coerce opcode works on FP, but fails on Ruffle due to dynamic scope lookup finding a non-type. So yeah they definitely need to ne decoupled first.

@Dinnerbone
Copy link
Contributor

Profiled the box2d demo before (red) and after (yellow) this change:
image

About 8% faster!

@Bale001 Bale001 force-pushed the scope-cache branch 4 times, most recently from b9df5b0 to 2022154 Compare February 4, 2023 03:26
@Bale001 Bale001 merged commit e7612f5 into ruffle-rs:master Feb 9, 2023
@Bale001 Bale001 deleted the scope-cache branch February 9, 2023 18:51
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.

3 participants