-
-
Notifications
You must be signed in to change notification settings - Fork 675
feat: optimize string literal tostack behavior #2945
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
Conversation
0461911 to
3a50dec
Compare
String literals are widely used static data, and our GC algorithm does not recycle them (see https://github.com/AssemblyScript/assemblyscript/blob/7e2a62d9615d2ae02b593af87ee4920a3c57b0bd/std/assembly/rt/itcms.ts#L244). Recording the addresses of all string literals and preventing the emission of tostack code can effectively improve performance
3a50dec to
6165368
Compare
In bootstrap parse, the execution time reduced from 210ms to 200ms.
2c2a0d8 to
4fb803f
Compare
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.
This optimization looks very promising (so cool that you found this!). I can definitely see the difficulty of making something equivalent to Set<i64> that works in the JS build.
My understanding of this PR is that you're trying to add the if (address < __heap_base) shortcut to the shadow stack generation code, and staticGcObjectOffsets is how you're trying to determine (for needToStack) whether some data is static data. Do you think it's possible to instead directly compare the address/offset to the value of __heap_base?
If this isn't easy or possible within the compiler, what would generating something like this (well, the isize equivalent) look like (assuming this idea is easy to do):
(if (i32.lt_u (local.get $address) (global.get $~lib/memory/__heap_base)) (; insert tostack code here ;) )
Would Binaryen optimize that out, even in debug builds? Just throwing some backup ideas out, because I'm not a huge fan of staticGcObjectOffsets (but if we can't do it any other way, I guess we have no choice).
My understanding of this PR is that you're trying to add the
if (address < __heap_base)shortcut to the shadow stack generation code, andstaticGcObjectOffsetsis how you're trying to determine (forneedToStack) whether some data is static data. Do you think it's possible to instead directly compare the address/offset to the value of__heap_base?
That is wrong. e.g. StaticArray also in data segments but we should add it to stack otherwise all of the A in this array will be free.
The precise description should be the so-called pointee free type located in the static data area. But string literal is the most common used type. So I also add string to the map. But we can extend it later.
Ah, understood.
Uh oh!
There was an error while loading. Please reload this page.
String literals are widely used static data, and our GC algorithm does not recycle them (see
assemblyscript/std/assembly/rt/itcms.ts
Line 244 in 7e2a62d