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

Improve native WebAssembly compilation #96

Closed
1 task done
CryZe opened this issue Jan 16, 2018 · 17 comments
Closed
1 task done

Improve native WebAssembly compilation #96

CryZe opened this issue Jan 16, 2018 · 17 comments
Labels
enhancement An improvement for livesplit-core.
Milestone

Comments

@CryZe
Copy link
Collaborator

CryZe commented Jan 16, 2018

Here's the current state of native WebAssembly compilation. Everything is working except:

@CryZe CryZe added enhancement An improvement for livesplit-core. help wanted labels Jan 16, 2018
@CryZe
Copy link
Collaborator Author

CryZe commented Jan 16, 2018

Looks like so far we are blocked by num not compiling due to rustc-serialize not compiling.

So we either have to wait for num to switch to serde rust-num/num#357
or rustc-serialize to actually merge rust-lang-deprecated/rustc-serialize#190 which seems actually pretty unlikely.

@CryZe
Copy link
Collaborator Author

CryZe commented Jan 24, 2018

With this PR chronotope/chrono#137 revived we could compile chrono on wasm too, we'd just need to bring our own Instant::now() and Utc::now()

@CryZe
Copy link
Collaborator Author

CryZe commented Jan 24, 2018

I began implementing this by not compiling the crates that don't work for the wasm target and polyfilling them. So that means mimicking relevant behavior (like parking_lot's locks being replaced by std's) and ignoring some other behavior for now (DateTimes are completely ignored, they are just empty structs)

@CryZe
Copy link
Collaborator Author

CryZe commented Jan 25, 2018

I got mostly everything working. Things that need to be fixed:

  • Folder structure changed a bit, CI scripts need to be adjusted
  • Instant::now()
  • Basic Image Display
  • PNG Decoding / Encoding
  • Utc::now()
  • Date Time calculations
  • Basic Colors
  • Color Conversions
  • Bindings
  • Hotkeys

CryZe added a commit that referenced this issue Jan 28, 2018
A lot of crates are polyfilled in the platform module. Most of the
polyfills work more or less correctly. Go to issue #96 for more
information.
@CryZe CryZe changed the title Explore native WebAssembly compilation without emscripten Improve native WebAssembly compilation Jan 28, 2018
@CryZe
Copy link
Collaborator Author

CryZe commented Jan 28, 2018

Looks like it was easy enough to just use the png crate to implement all the PNG Encoding / Decoding. So the only thing missing is chrono without the time dependency.

@CryZe
Copy link
Collaborator Author

CryZe commented Jan 28, 2018

Welp, it's actually somewhat broken. Whatever, at some point image will compile with the target, so I'm fine with that. It only really affects Llanfair files anyway.

@CryZe
Copy link
Collaborator Author

CryZe commented Jan 28, 2018

Nvm, turns out it was very easy to fix :)

@CryZe
Copy link
Collaborator Author

CryZe commented Feb 19, 2018

A recent upgrade of all the dependencies allowed us to get rid of all the polyfills except the one for chrono and the std::time stuff.

@CryZe
Copy link
Collaborator Author

CryZe commented Mar 6, 2018

Rust now uses LLD to link native wasm code, reducing the stack size from 1 MiB to 64 KiB. That's quite a large drop, considering we are using 5 MiB with emscripten. So we should probably check if everything still works and there's no stack overflows happening.

@hallcristobal
Copy link
Contributor

Are we still blocked by the chrono PR?

@CryZe
Copy link
Collaborator Author

CryZe commented Mar 6, 2018

Well we have everything stubbed out, but yes, the chrono PR doesn't seem to have progressed at all.

@CryZe
Copy link
Collaborator Author

CryZe commented Mar 6, 2018

Time to funnel contributors to chrono:
https://i.imgur.com/VJgt2xj.png

@CryZe
Copy link
Collaborator Author

CryZe commented Apr 7, 2018

This is now ready to be finished: chronotope/chrono#236 (comment)

@CryZe
Copy link
Collaborator Author

CryZe commented Apr 7, 2018

And it's still blocked by something. Apparently you can't have target specific features: rust-lang/cargo#1197

:(

@hallcristobal
Copy link
Contributor

That makes no sense. I don't see why you can't have target specific

@CryZe
Copy link
Collaborator Author

CryZe commented Apr 7, 2018

It's just not implemented correctly in cargo to support this properly atm.

@CryZe
Copy link
Collaborator Author

CryZe commented May 7, 2018

I'd really like to switch LiveSplit One over to the native wasm target (and potentially just throwing out the emscripten target for now). I've had to tell multiple people to just use the gh-pages branch to compile LiveSplit One, because setting up emscripten is just really annoying. So pushing for the native wasm target makes a ton of sense. It also should improve LiveSplit One's CPU utilization a bit (turning React into the bottleneck again).

So to push this further, we are really just blocked by chrono not really working out for us. We just can't disable the local feature selectively for the wasm target, because of the cargo bug. Therefore I'd like to just submit a PR to chrono that makes the wasm target have the same effect as disabling the local feature. The discussion about this is happening over here: chronotope/chrono#243

@CryZe CryZe closed this as completed in 615fa72 May 10, 2018
@CryZe CryZe added this to the v0.10 milestone Jun 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement for livesplit-core.
Projects
None yet
Development

No branches or pull requests

2 participants