Switch to WASMI engine in icu_codepointtrie_builder #4621

sffc · 2024-02-16T02:26:32Z

I switched our WASM engine to wasmi because it is faster to compile and has fewer dependencies. I changed how we interact with C++, too, in order to improve the performance.

sffc · 2024-02-16T22:00:58Z

Here is the output of depscheck with this PR:

Found two versions for `syn` (v1.0.109 & v2.0.48)
	Found non-allowlisted crate `ambient-authority`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `anyhow`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `async-trait`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `bitflags`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `cap-fs-ext`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `cap-primitives`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `cap-rand`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `cap-std`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `cap-time-ext`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `dirs`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `dirs-sys`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `downcast-rs`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `fs-set-times`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `getrandom`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `heck`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `indexmap-nostd`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `io-extras`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `io-lifetimes`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `ipnet`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `is-terminal`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `leb[128](https://github.com/unicode-org/icu4x/actions/runs/7929915708/job/21651128951?pr=4621#step:11:129)`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `linux-raw-sys`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `maybe-owned`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `paste`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `pin-project-lite`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `ppv-lite86`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `rand`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `rand_chacha`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `rand_core`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `rustix`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `shellexpand`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `spin`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `system-interface`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `tracing`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `tracing-attributes`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `tracing-core`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `wasi-cap-std-sync`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `wasi-common`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `wasmi`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `wasmi_arena`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `wasmi_core`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `wasmi_wasi`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `wasmparser-nostd`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `wast`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `wiggle`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `wiggle-generate`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `wiggle-macro`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional
	Found non-allowlisted crate `witx`, consider adding to `EXTRA_DATAGEN_DEPS` or `EXTRA_ZIP_DEPS` or `EXTRA_RAYON_DEPS` in tools/depcheck/src/allowlist.rs if intentional

@Manishearth

sffc · 2024-02-16T22:27:51Z

On main:

INFO  [icu_datagen::driver] Generated key segmenter/sentence@1 (3.360s, flushed in 64.444ms)
INFO  [icu_datagen::driver] Generated key segmenter/word@1 (4.779s, flushed in 62.042ms)
INFO  [icu_datagen::driver] Generated key segmenter/grapheme@1 (4.805s, flushed in 55.470ms)
INFO  [icu_datagen::driver] Generated key segmenter/line@1 (9.127s, flushed in 79.752ms)

On this PR:

INFO  [icu_datagen::driver] Generated key segmenter/grapheme@1 (6.838s, flushed in 55.157ms)
INFO  [icu_datagen::driver] Generated key segmenter/sentence@1 (8.447s, flushed in 63.641ms)
INFO  [icu_datagen::driver] Generated key segmenter/word@1 (8.472s, flushed in 61.680ms)
INFO  [icu_datagen::driver] Generated key segmenter/line@1 (29.712s, flushed in 78.175ms)

Please let me know what you all think. I don't want to sink more time into this until we know which direction we're taking.

Manishearth

I'm happy with this approach. I'm okay with the slowdown, though I think we can potentially speed things up by building via ranges instead of codepoint by codepoint.

robertbastian · 2024-02-19T12:23:14Z

components/collections/codepointtrie_builder/Cargo.toml

+wasmi = "0.31.2"
+wasmi_wasi = "0.31.2"


issue: why are these non-optional?

components/collections/codepointtrie_builder/cpp/README.md

robertbastian · 2024-02-19T12:27:32Z

components/collections/codepointtrie_builder/src/lib.rs

-        clippy::unwrap_used,
-        clippy::expect_used,
-        clippy::panic,
+        // This Datagen-only builder code may panic when interacting with WASM


nit: datagen is not really a concept in this crate

Updated the docs

sffc · 2024-02-24T01:22:11Z

I managed to get rid of the WASI requirement at runtime entirely, which eliminates a ton of dependencies. We still need WASI for building the C++ code, but the linker strips out all of the runtime WASI symbols. This is enabled by Reactor Modules which are a relatively recent change to the WASI spec.

components/collections/codepointtrie_builder/cpp/Makefile

robertbastian · 2024-02-26T09:37:24Z

components/collections/codepointtrie_builder/src/wasm.rs

+        2 => 0, // UCPTRIE_VALUE_BITS_16
+        4 => 1, // UCPTRIE_VALUE_BITS_32
+        1 => 2, // UCPTRIE_VALUE_BITS_8


robertbastian · 2024-02-26T09:38:08Z

components/collections/codepointtrie_builder/ucptrie_wrap.wasm

impressive size reduction!

Manishearth

also hooray we don't need to dedup syn anymore

either way i have most of the work done for that in wasmi-labs/wasmi#941

Rough upgrade to Wasmer 4

2773ec7

sffc changed the title ~~Rough upgrade to Wasmer 4~~ Upgrade to Wasmer 4 Feb 16, 2024

sffc requested a review from robertbastian February 16, 2024 02:26

This comment was marked as outdated.

Sign in to view

sffc added 5 commits February 15, 2024 23:42

Try out wasmi

6fcd30d

Checkpoint: wasmi without stdio

978f414

Make it a whole lot faster

1b3dd98

Cleanup

38a33c3

Delete and move code and artifacts

34efb1c

sffc changed the title ~~Upgrade to Wasmer 4~~ Switch to WASMI engine in icu_codepointtrie_builder Feb 16, 2024

sffc added 3 commits February 16, 2024 03:21

Cargo.toml cleanup

7144e56

Fix license header

c48cf69

Clippy

769aa5d

sffc marked this pull request as ready for review February 16, 2024 11:30

sffc requested review from echeran and a team as code owners February 16, 2024 11:30

sffc requested review from Manishearth and removed request for echeran and robertbastian February 16, 2024 11:30

Manishearth previously approved these changes Feb 17, 2024

View reviewed changes

jedel1043 mentioned this pull request Feb 19, 2024

RUSTSEC-2024-0014: generational-arena is unmaintained boa-dev/boa#3681

Closed

robertbastian reviewed Feb 19, 2024

View reviewed changes

Remove WASI requirement at runtime!

875c5f5

sffc dismissed Manishearth’s stale review via 875c5f5 February 24, 2024 01:18

sffc added 2 commits February 23, 2024 17:28

Optional dependency

7c501c0

Panic docs

6331cd1

readme

b44aa99

robertbastian previously approved these changes Feb 26, 2024

View reviewed changes

sffc added 2 commits February 27, 2024 01:46

Move weird common code into module

6ea6947

Clang flag comments

481bd80

sffc dismissed robertbastian’s stale review via 481bd80 February 27, 2024 09:54

Fix native build

bb20726

robertbastian approved these changes Feb 27, 2024

View reviewed changes

Manishearth approved these changes Feb 27, 2024

View reviewed changes

sffc merged commit 952a39f into unicode-org:main Feb 27, 2024
30 checks passed

sffc deleted the wasmer4 branch February 27, 2024 16:42

This was referenced Feb 27, 2024

move to wasmer 3.1.1 #3367

Closed

icu_codepointtrie_builder optimizations #3417

Closed

Unignore testdata test and make it hermetic #4317

Merged

Improve performance of icu_codepointtrie_builder #1884

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to WASMI engine in icu_codepointtrie_builder #4621

Switch to WASMI engine in icu_codepointtrie_builder #4621

sffc commented Feb 16, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

sffc commented Feb 16, 2024

sffc commented Feb 16, 2024

Manishearth left a comment

robertbastian Feb 19, 2024

sffc Feb 24, 2024

robertbastian Feb 19, 2024

sffc Feb 24, 2024

sffc commented Feb 24, 2024

robertbastian Feb 26, 2024

robertbastian Feb 26, 2024

Manishearth left a comment

Switch to WASMI engine in icu_codepointtrie_builder #4621

Switch to WASMI engine in icu_codepointtrie_builder #4621

Conversation

sffc commented Feb 16, 2024 • edited Loading

This comment was marked as outdated.

This comment was marked as outdated.

sffc commented Feb 16, 2024

sffc commented Feb 16, 2024

Manishearth left a comment

Choose a reason for hiding this comment

robertbastian Feb 19, 2024

Choose a reason for hiding this comment

sffc Feb 24, 2024

Choose a reason for hiding this comment

robertbastian Feb 19, 2024

Choose a reason for hiding this comment

sffc Feb 24, 2024

Choose a reason for hiding this comment

sffc commented Feb 24, 2024

robertbastian Feb 26, 2024

Choose a reason for hiding this comment

robertbastian Feb 26, 2024

Choose a reason for hiding this comment

Manishearth left a comment

Choose a reason for hiding this comment

sffc commented Feb 16, 2024 •

edited

Loading