-
Notifications
You must be signed in to change notification settings - Fork 197
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
New hash scheme #555
New hash scheme #555
Conversation
Some questions:
|
Cool! |
New commits produce even less whitespace in generated css for faster hashing and dict lookups. |
@robinheghan Looks like there are some merge conflicts since I merged your other PR. 😅 |
I’ll fix it :) |
the Css is used as a dictionary key now. In theory, this makes it so that we only hash unique Css styles, and we only perform the elm-css -> actual-css conversion once + some string replacement.
…up as hotspots in benchmarks.
…it's shorter and just as unlikely to come up in css.
All tests pass, ready to merge :) |
This PR makes a big change to how classNames are generated.
Previously the flow worked something like this:
There are some potential performance improvements to this flow:
This PR tries to improve performance by:
String.replace
on the compiled css, replacing the template selector with the hashed class name.Somewhat unrelated, this PR also replaces a bunch of
String.join
calls with direct string concatination (++) where possible, asString.join
shows up as a hot spot when profiling.Depending on the browser, this change represents a 18-30% improvement compared to the current master branch.