core: Remove all static strings, shrink Value structs #19716
Conversation
Lord-McSweeney
added
A-avm2
| @@ -595,8 +548,6 @@ pub fn load_player_globals<'gc>( | |||
|
|
|||
| globals.set_vtable(mc, global_obj_vtable); | |||
|
|
|||
| activation.context.avm2.toplevel_global_object = Some(globals); | |||
There was a problem hiding this comment.
To be sure, why was this moved later?
There was a problem hiding this comment.
We have three script global objects right now- one for the builtin classes script, one for Toplevel.as, and one for globals.as. The "toplevel" global object has pointed to the builtin one for a long time, but as we moved classes out of it and into Toplevel.as it makes more sense to make it point to the Toplevel.as global object.
The hacky part was that some tests depended on the "toplevel" global object having the parseInt, parseFloat, etc properties, which, since they were defined in Toplevel.as, we had to copy over to the builtin global object manually. Now that the only properties left on the builtin global object are Object and Class anyway, I made the "toplevel" global object point to Toplevel.as and removed the copying hack.
| @@ -290,7 +290,7 @@ impl<'gc> Domain<'gc> { | |||
| activation.gc(), | |||
| &name[(start + 2)..(name.len() - 1)], | |||
| )); | |||
| name = "__AS3__.vec::Vector".into(); | |||
| name = AvmString::new_utf8(activation.gc(), "__AS3__.vec::Vector"); | |||
There was a problem hiding this comment.
This could still be an AvmString-pointing-to-static, right?
| @@ -100,8 +103,9 @@ pub fn create_c_class<'gc>( | |||
| let gc_context = activation.gc(); | |||
| let namespaces = activation.avm2().namespaces; | |||
|
|
|||
| let class_name = AvmString::new_utf8(gc_context, "Class$"); | |||
There was a problem hiding this comment.
Managed-pointing-to-static?
|
So a lot of these Also after this gets merged, I'll do an instrumented test to see what dynamic strings we now allocate from our own code during normal runtime. |
Lord-McSweeney
force-pushed
the
no-istr
branch
2 times, most recently
from
17c9bc0 to
45c741e
Compare
| const _: () = assert!(size_of::<Value<'_>>() <= 16); | ||
|
|
||
| #[cfg(target_pointer_width = "64")] | ||
| const _: () = assert!(size_of::<Value<'_>>() == 24); |
There was a problem hiding this comment.
Can you keep them separate, to be more explicit? Will be also useful if we ever do NaN boxing on 32-bit.
| @@ -38,12 +38,8 @@ impl Debug for Error<'_> { | |||
| } | |||
|
|
|||
| // This type is used very frequently, so make sure it doesn't unexpectedly grow. | |||
| #[cfg(target_family = "wasm")] | |||
| const _: () = assert!(size_of::<Result<Value<'_>, Error<'_>>>() == 24); | |||
There was a problem hiding this comment.
I thought that the size is smaller (16?) on non-wasm due to x86 being less strict with alignment?
At least at some point it was. Do we do any testing on 32-bit?
That's why I suggest keeping them separate here too.
…EventObject::io_error_event
AvmStringnow directly holds aGc<AvmStringRepr>, which shrinks Value structs on 64-bit from 24 to 16 bytes. getlocal, setlocal, and getslot run 5-10% faster; Chess Demons starts up about 5% faster.