Starlight-JS / starlight

JS engine in Rust
https://teletype.in/@starlight-js
Mozilla Public License 2.0
511 stars 9 forks source link

async/promise PoC #65

Closed andrieshiemstra closed 3 years ago

andrieshiemstra commented 3 years ago

This is more a review request than a final PR

jameslahm commented 3 years ago

serializer seems to be broken. Promise native methods should be add in VM_NATIVE_REFERENCES. https://github.com/Starlight-JS/starlight/blob/4b49db82ee017a4ff82029a9b4a1f9a9dbc6b5ca/crates/starlight/src/jsrt.rs#L901-L913

andrieshiemstra commented 3 years ago

serializer seems to be broken. Promise native methods should be add in VM_NATIVE_REFERENCES.

Thanks for pointing that out! fixed

playXE commented 3 years ago

for persistent roots i use a HashMap based on a generated ID, at first i thought i'd just use the inner ptr of JsObejct as ID but then it would not work when multiple jobs add a persistent root for the same object.

Your persistent root can be reference counted and stored in hash map. On GC when constraint for the Runtime instance is executed you just remove all the roots that have 1 reference count left. I think you can even just use Vec there?

playXE commented 3 years ago

BTW maybe you can remove async from test_ignore.txt so we can see how many tests run fine and what tests might segfault?

andrieshiemstra commented 3 years ago

remove async from test_ignore.txt

i'l work on this seperately > #66

andrieshiemstra commented 3 years ago

fairly happy about this for now, since i'm on vacation the next 3 weeks i'd like to merge this now, there are some followup tickets i'll create to get Promises realy fully functional