Skip to content

Refactor and simplify asyncify logic for calling back in #12081

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

Merged
merged 1 commit into from
Aug 31, 2020
Merged

Conversation

kripken
Copy link
Member

@kripken kripken commented Aug 29, 2020

This removes callStackIdToFunc, the map of ids to functions. Instead it
makes us always use the map of ids to function names. That removes one
of the two mechanisms and makes the code simpler, which will help in a
followup PR that adds support for direct table calls, which we need for
#12059.

I think we kept both mechanisms because I was worried about performance,
but I think I was wrong. After debugging for #12059 I think I have a better
understanding of things and it seems fine to use names - it's just a single
JS map lookup for asyncify operation, which is small compared to the
actual asyncify overhead.

Move some code to maybeStopUnwind, with no changes. That code
will be useful in the followup as well.

Also clean up some debugging code to make it more useful.

@Akaricchi please take a look, thanks!

Copy link
Contributor

@Akaricchi Akaricchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surprisingly the koishi context switching microbench is actually a bit faster with these changes on average

20 runs on this commit (under node):

10000000 switches in 6.418574s  ~1557978.463143 switches/s      ~641 ns/switch
10000000 switches in 6.157174s  ~1624121.602419 switches/s      ~615 ns/switch
10000000 switches in 6.314802s  ~1583580.912419 switches/s      ~631 ns/switch
10000000 switches in 6.287445s  ~1590471.135235 switches/s      ~628 ns/switch
10000000 switches in 6.146073s  ~1627055.297146 switches/s      ~614 ns/switch
10000000 switches in 6.036533s  ~1656580.142787 switches/s      ~603 ns/switch
10000000 switches in 6.400194s  ~1562452.555152 switches/s      ~640 ns/switch
10000000 switches in 6.367633s  ~1570442.384023 switches/s      ~636 ns/switch
10000000 switches in 6.345709s  ~1575868.017975 switches/s      ~634 ns/switch
10000000 switches in 6.115042s  ~1635311.750000 switches/s      ~611 ns/switch
10000000 switches in 6.460434s  ~1547883.557590 switches/s      ~646 ns/switch
10000000 switches in 6.040741s  ~1655425.925674 switches/s      ~604 ns/switch
10000000 switches in 6.282504s  ~1591722.057668 switches/s      ~628 ns/switch
10000000 switches in 5.974481s  ~1673785.531786 switches/s      ~597 ns/switch
10000000 switches in 6.305687s  ~1585869.947234 switches/s      ~630 ns/switch
10000000 switches in 6.205425s  ~1611493.122279 switches/s      ~620 ns/switch
10000000 switches in 6.211860s  ~1609823.705500 switches/s      ~621 ns/switch
10000000 switches in 6.185903s  ~1616578.771864 switches/s      ~618 ns/switch
10000000 switches in 6.230681s  ~1604960.965931 switches/s      ~623 ns/switch
10000000 switches in 6.026554s  ~1659323.054001 switches/s      ~602 ns/switch

20 runs on previous commit:

10000000 switches in 6.554317s  ~1525712.018926 switches/s      ~655 ns/switch
10000000 switches in 6.547642s  ~1527267.452346 switches/s      ~654 ns/switch
10000000 switches in 6.230464s  ~1605016.844035 switches/s      ~623 ns/switch
10000000 switches in 6.461598s  ~1547604.844549 switches/s      ~646 ns/switch
10000000 switches in 6.677223s  ~1497628.690600 switches/s      ~667 ns/switch
10000000 switches in 6.304400s  ~1586193.828557 switches/s      ~630 ns/switch
10000000 switches in 6.369800s  ~1569908.098279 switches/s      ~636 ns/switch
10000000 switches in 6.364358s  ~1571250.311150 switches/s      ~636 ns/switch
10000000 switches in 6.397517s  ~1563106.341894 switches/s      ~639 ns/switch
10000000 switches in 6.289302s  ~1590001.485286 switches/s      ~628 ns/switch
10000000 switches in 6.374446s  ~1568763.726225 switches/s      ~637 ns/switch
10000000 switches in 6.231919s  ~1604642.080628 switches/s      ~623 ns/switch
10000000 switches in 6.541209s  ~1528769.278122 switches/s      ~654 ns/switch
10000000 switches in 6.271082s  ~1594621.150230 switches/s      ~627 ns/switch
10000000 switches in 6.208038s  ~1610814.951533 switches/s      ~620 ns/switch
10000000 switches in 6.231353s  ~1604787.815120 switches/s      ~623 ns/switch
10000000 switches in 6.499706s  ~1538531.034382 switches/s      ~649 ns/switch
10000000 switches in 6.847392s  ~1460409.986711 switches/s      ~684 ns/switch
10000000 switches in 6.166493s  ~1621667.375696 switches/s      ~616 ns/switch
10000000 switches in 6.325251s  ~1580964.966039 switches/s      ~632 ns/switch

Tentatively LGTM from me. I have yet to see if this breaks anything in Taisei, but the code changes look fine.

Overall the performance is of course still disappointing compared to even the slowest native backend (ucontext). Recently I've discovered that Firefox suffers especially badly; Taisei's master branch gets unplayable with about a hundred fiber switches per frame there. Looking forward to seeing the impact of direct table calls on this.

@kripken
Copy link
Member Author

kripken commented Aug 31, 2020

Thanks @Akaricchi , interesting it is slightly faster. Maybe the simpler JS helps the JIT somehow.

Overall the performance is of course still disappointing compared to even the slowest native backend (ucontext).

Yeah, that is still not going to be great. I think the only real solution is a wasm spec addition. There is ongoing design work on wasm stack switching which should provide what we need.

@kripken kripken merged commit 4ae9f49 into master Aug 31, 2020
@kripken kripken deleted the async1 branch August 31, 2020 17:16
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