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

Wasm support #135

Closed
hallucino opened this issue Feb 1, 2018 · 17 comments
Closed

Wasm support #135

hallucino opened this issue Feb 1, 2018 · 17 comments

Comments

@hallucino
Copy link

Hi,

I'm trying to build a pure wasm version of UnicornConsole (https://github.com/Gigoteur/UnicornConsole) which use syntect for code editing. But right now I'm getting an error with same-file crate dependency:
--> /home/xx/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/same-file-0.1.3/src/lib.rs:90:9
|
90 | imp::Handle::from_path(p).map(Handle)
| ^^^ Use of undeclared type or module imp
error[E0433]: failed to resolve. Use of undeclared type or module imp
--> /home/xx/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/same-file-0.1.3/src/lib.rs:95:9

I'm opening the issue here to understand if you need this dependency or not, because it seems not really needed ?

Thank you

@trishume
Copy link
Owner

trishume commented Feb 2, 2018

Yah this isn't viable yet, but maybe soon. There's a few dependencies that rely on C. The only one that's difficult to get out is fancy-regex, and that's being worked on in #34. Once fancy-regex works though, then it shouldn't be too hard to wrangle the rest of the dependencies to work in WASM.

@FreeMasen
Copy link

FreeMasen commented Feb 28, 2018

@trishume It looks like a big part would be the use of WalkDir which depends on same-file which is where the above error is coming form. Could you allow the removal of ThemeSet:discover_theme_paths and ThemeSet::load_from_folder functions on a feature flag?

maybe something like #[cfg(not(all(target = "wasm32", target_os = "unknown")))]

@trishume
Copy link
Owner

trishume commented Mar 1, 2018

@FreeMasen I could when fancy-regex arrives, but right now it wouldn't do much good since then you'll just hit a different error about Oniguruma that won't have an easy fix.

@k3d3
Copy link

k3d3 commented Apr 23, 2020

Now that fancy-regex is implemented according to #270, is this issue still on the radar?

@trishume
Copy link
Owner

Dunno, it might just work™ now with wasm. If someone tries it and confirms that it works I can close this issue, but hopefully it does as long as you use the fancy-regex cargo feature and maybe disable some other cargo features that might load C things but aren't necessary if using internal dumps, like yaml and theme loading.

@alexpeattie
Copy link

It seems it does compile indeed 👍 . For reference I just cloned the repo, ran:

rustup target add wasm32-unknown-unknown

Changed the default feature in Cargo.toml to "default-fancy", and added:

[lib]
crate-type = ["cdylib", "rlib"]

and then ran:

cargo build --target wasm32-unknown-unknown

which runs with no errors. The next steps I'd like to play around with would be to start adding WASM bindings and create a minimal example of highlighting in the browser.

@alexpeattie
Copy link

Update: I got a little end-to-end demo of syntect highlighting running in the browser:

https://syntect-wasm-demo.alexpeattie.com/

The source is here. Everything worked great, it was all very easy to set up 👏 ! I think this issue can be closed probably.

@trishume - would you be interested in adding Wasm bindings to syntect at all? Or would you prefer to have some 3rd party bindings implemented elsewhere?

@trishume
Copy link
Owner

@alexpeattie Nice demo!

I think given that there's a bunch of different ways you might want to use syntect in wasm, and it should be doable as a separate crate, it probably makes more sense for it to be a different repo/library. However if anyone makes bindings I'm happy to link to them in the syntect Readme!

@ranile
Copy link

ranile commented Sep 23, 2020

That works but currently this library, when theme set and syntax set are loaded (with ThemeSet::load_defaults() and SyntaxSet::load_defaults_nonewlines() respectively), the size of the library just gets too big. I have had it go from ~590KB to ~2MB by just loading those. I hope there is a solution to this problem because otherwise, this library is basically useless when shipping to client side web app is required.

@keith-hall
Copy link
Collaborator

Without the assets feature enabled, you could choose what syntaxes and color schemes to include to reduce bundle size / load them separately from the library. The dumps are compressed already, so I'm not sure what else can be done except individually load and parse the original files on demand at the cost of performance I guess.

@ranile
Copy link

ranile commented Sep 28, 2020

Without the assets feature enabled, you could choose what syntaxes and color schemes to include to reduce bundle size / load them separately from the library. The dumps are compressed already, so I'm not sure what else can be done except individually load and parse the original files on demand at the cost of performance I guess.

@keith-hall, From what I can tell, the themes and syntax set are inside a single binary. It doesn't seem like individually load syntax sets and themes. Can you link me any documentation/example(s) for this?

@keith-hall
Copy link
Collaborator

https://docs.rs/syntect/4.4.0/syntect/parsing/struct.SyntaxSetBuilder.html#impl
https://docs.rs/syntect/4.4.0/syntect/parsing/syntax_definition/struct.SyntaxDefinition.html#impl e.g. load_from_str would allow you to make a fetch request for a .sublime-syntax file, add it to a SyntaxSetBuilder etc.
https://docs.rs/syntect/4.4.0/syntect/highlighting/struct.ThemeSet.html#impl-1 - see i.e. load_from_folder or get_theme

@trishume
Copy link
Owner

Yah I think the way to go for WASM is to build a custom syntax pack file with only the syntaxes you need. You may also want to experiment with both compressing and not compressing it to save the size of the decompression in the WASM. Or depending on your app making multiple pack files and loading the required ones on demand. Then the build for WASM can also disable the cargo features for parsing the text formats to save binary size.

@ranile
Copy link

ranile commented Oct 4, 2020

That defiantly helps with the size of the built output but the performance is slow when using regex-fancy. With the current version of syntect, that is a requirement as the current version of rust-onig, v6.0.0, used by syntect fails to compile on wasm target but with the latest version v6.1.0, there was attempt to add support for WASM (see: rust-onig/rust-onig#147).

I have not tested if these changes solves the compilation error that occurs when compiling syntect on wasm target and therefore I also do not know if the performance will be improved and if so, by how much. But going by what readme states that the performance is halved when using regex-fancy compared to regex-onig, it could/should be fast be to be run on main thread without any noticeable blocking which, in my eyes, is a welcome and useful improvement.

@ranile
Copy link

ranile commented Oct 5, 2020

Update: updating to v6.1.0 did not help so I created this rust-onig/rust-onig#153

@jkelleyrtp
Copy link

jkelleyrtp commented Jul 21, 2021

That defiantly helps with the size of the built output but the performance is slow when using regex-fancy. With the current version of syntect, that is a requirement as the current version of rust-onig, v6.0.0, used by syntect fails to compile on wasm target but with the latest version v6.1.0, there was attempt to add support for WASM (see: rust-onig/rust-onig#147).

I have not tested if these changes solves the compilation error that occurs when compiling syntect on wasm target and therefore I also do not know if the performance will be improved and if so, by how much. But going by what readme states that the performance is halved when using regex-fancy compared to regex-onig, it could/should be fast be to be run on main thread without any noticeable blocking which, in my eyes, is a welcome and useful improvement.

Using fancy_regex is terribly terribly slow, it takes about 100ms to highlight around 80 lines of Rust split over 6 code blocks.

For each code block I'm creating a new ParseState, but I see that they can cached. How does that work? Can I keep the regex's compiled instead of rebuilding them for each block?

In its current state is pretty much unusably slow in the browser. Either some serious caching would need to kick in, some sort of pre-rendering, or just switching to something like prism or highlighter.js which have a simpler less-robust take on highlighting.

@Enselic
Copy link
Collaborator

Enselic commented Nov 10, 2021

That was a super cool demo!

I'm sure there are opportunities for more improvements in this area, but since the original "Wasm support" problem is not around any more, I think it makes perfect sense to close this issue now, as discussed in #135 (comment) and #135 (comment)

Any follow-up problems are better handled in individual issues with to-the-point descriptions.

@Enselic Enselic closed this as completed Nov 10, 2021
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

No branches or pull requests

9 participants