Migrate to emscripten

[jeetiss]

No description provided.

[jeetiss]

@NickGerleman hello, I'm back

The bindings are ready so let's discuss the migration and the next steps.

• I just replaced nbind with embind and removed all nbind code. Is it okay or it would be better to maintain both versions while migrating?

• I wanna provide separate dev (with some additional assertions and checks) and prod (minified and with optimizations) bundles, so I need some bundler (rollup). What do you think about that? maybe you have suggestions here.

• I don't know what to do with CI. Can I setup GitHub action for testing like this or it is not a big deal rn?

[NickGerleman]

This is incredible! I will take a close look soon.

I think it is okay to fully replace nbind. It has been a long time since folks used it for Node native module versions of Yoga compared to packages like yoga-layout-prebuilt, so it is better imo to have something working, compared to full parity.

Re dev vs prod, I know Emscripten has options for what it emits (it will already minify), so I’m guessing the extra bits would be for the JavaScript frontend to it? If we can avoid the complexity, it might be better to distribute the package without running through another bundler, then the consuming project could bundle using the bundler they are already using.

I’d love it if we could add GitHub Actions validation, but I know it isn’t present right now. There are some workflows I added recently that could be pattern matched, and it would document how we are running tests against this. The website build is currently using the last published version of yoga-layout instead of the one in the repo too, so we don’t get validation from the website build job (tried to move this to workspaces recently but the website setup needs some serious renovation to support it).

[jeetiss]

I got the warning

warning: undefined symbol: _ZN8facebook4yoga24LayoutPassReasonToStringENS0_16LayoutPassReasonE (referenced by top-level compiled C/C++ code)

do you have any idea why it is here and maybe it can be fixed

[NickGerleman]

I got the warning

warning: undefined symbol: _ZN8facebook4yoga24LayoutPassReasonToStringENS0_16LayoutPassReasonE (referenced by top-level compiled C/C++ code)

do you have any idea why it is here and maybe it can be fixed

I think this means the build isn’t picking up yoga/event/event.cpp.

[NickGerleman]

I'm super grateful for this work. Given how the nbind version was long-term broken, we should be aggressive in taking improvements here.

If you don't mind, I'm going to try to push a commit to this PR adding the yarn test script you wrote to GitHub workflows. The previous lack of coverage for bindings let a lot of bad code slip into them, so if we have a working test script, it would help to both protect the change and demonstrate it is working when I import it 🙂.

[NickGerleman]

I pushed a couple of changes here:

1. We have passing tests in GitHub Actions now 🎉
2. We build separate targets for asm.js and wasm.js, defaulting to asm.js for compat, but allowing platforms supporting wasm to use the wasm entrypoint from the export map (if using a modern bundler or Node). Anecdotally, the WASM build of tests ran in about half the time of the asm.js version.
3. Updated the babel versions, transforms. The old version didn't recognize Flow syntax to reexport all types, which I wanted to add to avoid duplication in the multiple entrypoints.

[NickGerleman]

Here is the performance difference between asm.js and wasm being run by Node from the scenarios in the run-bench script.

[nicoburns]

Here is the performance difference between asm.js and wasm being run by Node from the scenarios in the run-bench script.

Interesting. I'm not sure how representative these benchmarks are, but ~13ms for the "huge nested layout" was also what I was getting by using the yoga-layout-prebuilt package from NPM (although I was only measuring layout computation time and not including the tree setup time there).

I can't understand why "Align stretch in undefined axis" is so slow though. That only has 10 nodes (compared to 10,000 for the huge nested layout)! Admittedly it's not the same benchmark, but I was able to run a variant of the "huge nested layout" with depth of 1 instead of depth of 4 (so 10^1 = 10 rather than 10^4 = 10,000 nodes) in ~45 micro seconds.

One thing I did notice when benchmarking yoga-layout-prebuilt was that there seemed to be a fixed ~30ms penalty the first time I called calculate_layout() (can't remember exact name in yoga) in a given run (this happened even if other functions to setup nodes had already been called. So in order to get accurate results I had to run the benchmark once and throw away the result before collecting measurements. I wonder if a similar penalty might be skewing the results here. I would say that the results of the 4 benchmarks are suspiciously similar.

[NickGerleman]

I haven't tried to debug it to see if it is working as expected, but there is some mention in the script on skipping the first few iterations to avoid one-time costs, then averaging the rest.

Some of the tests I can see like "Align stretch in undefined axis" add 2000 child nodes under the root node, so there still is a lot there. That 2000 node count is the same for other tests, but "huge nested layout" is triply nested of 6 children it looks like.

[facebook-github-bot]

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@NickGerleman merged this pull request in 1813748.