avm1: Dynamically resolve builtin classes (1/N: preliminaries) - #24327
avm1: Dynamically resolve builtin classes (1/N: preliminaries)#24327moulins wants to merge 3 commits into
Conversation
|
I think there was a discussion about it with @adrian17 at some point because we were worried it will negatively impact performance by a lot. There was this idea to keep |
| v = _global.Object; | ||
|
|
||
| trace("// Object = {}"); | ||
| _global.Object = {}; |
There was a problem hiding this comment.
What if Object is a virtual property? We could add a getter here and see exactly when FP resolves classes.
There was a problem hiding this comment.
The monkeyPatch function adds a getter, so this is already tested afterwards.
I just wrote this micro-benchmark: Code var i = 1000;
var o;
while (i--) {
o = {}; o = {}; o = {}; o = {};
o = {}; o = {}; o = {}; o = {};
o = {}; o = {}; o = {}; o = {};
o = {}; o = {}; o = {}; o = {};
o = {}; o = {}; o = {}; o = {};
}Results
There is some performance loss (roughly -33%), but I think it's acceptable, as:
There are also some avenues to improve object lookup performance in the future, e.g. in case sensitive mode we could store keys as interned strings to get cheaper hashing and equality tests in |
09796c7 to
2ba925b
Compare
|
Update: I did further tests on more realistic code, and any performance cost gets lost in the noise. |
Unlike what Ruffle implements (with the `SystemPrototypes` struct), Flash Player appears to always resolve classes on the global object, allowing for builtins to be monkeypatched at runtime. For now, this only tests a small part of the API surface, but this will be expanded upon in the future.
...instead of using the pre-cached object prototype.
This is needed to properly distinguish between no `prototype` and `prototype = undefined` when resolving builtin classes.
2ba925b to
f07aa5a
Compare
Description
When Ruffle's native code needs to instantiate an AVM1 object, it selects the prototype to use from the static list stored in the
SystemPrototypesstruct.However, this is incorrect, as testing in FP shows that overwriting builtin classes in the
_globalscope affects the prototype of objects instantiated by the player.This PR lays the foundations for this change by:
Activation::resolve_prototypemethod, to lookup a class prototype on the global scopeInitObjectaction to rely on the above method instead ofSystemPrototypes, as a proof of concept.Follow-up PRs will switch more instantiations to use
resolve_prototype; in the long term I expect theSystemPrototypesstruct to be entirely phased out.Testing
The added test (
avm1/globals_monkeypatch) only exercises a few examples, it will be expanded upon in future PRs.Checklist