Skip to content
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

Make standard library dependencies pluggable #3

Closed
sffc opened this issue Feb 29, 2020 · 6 comments
Closed

Make standard library dependencies pluggable #3

sffc opened this issue Feb 29, 2020 · 6 comments
Assignees
Labels
A-ffi Area: FFI, WebAssembly, Transpilation C-meta Component: Relating to ICU4X as a whole R-obsolete Resolution: This issue is no longer relevant T-enhancement Type: Nice-to-have but not required

Comments

@sffc
Copy link
Member

sffc commented Feb 29, 2020

In order to reduce code size when shipping binaries to other environments, such as WebAssembly, it may be desirable to give a way to leverage the environment's standard library instead of building the Rust version into the OmnICU binary.

For example, JavaScript has a Map type. If we build OmnICU to target WebAssembly, it would be nice if OmnICU could import JavaScript Map instead of shipping hashbrown.

CC @kripken

@kripken
Copy link

kripken commented Feb 29, 2020

In general this sounds good for code size, but doing it on a core data structure seems risky for perf due to many wasm<->JS calls (imagine iteration on a map).

cc @alexcrichton @fitzgen who might have thoughts or be aware of relevant work.

@Manishearth
Copy link
Member

The cost tradeoffs are definitely weird there, but we could potentially have this cfg-based

@fitzgen
Copy link

fitzgen commented Mar 5, 2020

It is possible to do this, and I would suggest making a crate which has as similar an api as std::collections::HashMap or whatever as possible. No one has published anything like this yet, afaik, but not because it isn't possible. Yes, cost trade offs, as mentioned above. Also implies depending on some extra tool to hook up imports of the js map type (eg wasm-bindgen and js-sys)

@sffc
Copy link
Member Author

sffc commented Apr 16, 2020

Action: this should be discussed as part of the Rust Style Guide (#12).

@sffc sffc self-assigned this Apr 16, 2020
@sffc sffc added the T-docs-tests Type: Code change outside core library label Apr 16, 2020
@sffc sffc added A-ffi Area: FFI, WebAssembly, Transpilation C-meta Component: Relating to ICU4X as a whole T-docs-tests Type: Code change outside core library and removed T-docs-tests Type: Code change outside core library labels May 7, 2020
@sffc sffc added this to the 2020 Q2 milestone Jun 17, 2020
@sffc
Copy link
Member Author

sffc commented Jun 25, 2020

This issue is not immediately actionable until we start hooking up FFI and WebAssembly. I'm documenting in the style guide as part of #77 to favor alternatives to HashMap where possible. We should revisit this issue when more code is written and evaluate the cost/benefit tradeoff.

@sffc sffc removed this from the 2020 Q2 milestone Jun 25, 2020
@sffc sffc closed this as completed Jun 25, 2020
@sffc sffc reopened this Sep 4, 2020
@sffc
Copy link
Member Author

sffc commented Apr 3, 2021

We now have LiteMap (#496), which solves the HashMap problem. It's not clear that there are any other major areas where we would need this functionality. I will therefore close this issue as obsolete.

@sffc sffc closed this as completed Apr 3, 2021
@sffc sffc added R-obsolete Resolution: This issue is no longer relevant T-enhancement Type: Nice-to-have but not required and removed T-docs-tests Type: Code change outside core library backlog labels Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ffi Area: FFI, WebAssembly, Transpilation C-meta Component: Relating to ICU4X as a whole R-obsolete Resolution: This issue is no longer relevant T-enhancement Type: Nice-to-have but not required
Projects
None yet
Development

No branches or pull requests

4 participants