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

Small performance tweak (primarily for start time) #3276

Closed

Conversation

nazar-pc
Copy link
Contributor

@nazar-pc nazar-pc commented Jan 7, 2016

Small performance tweak (primarily for start time) to avoid duplicated styles injections (once skipping CSS mixins and variables and second time with them)

Besides it allows to skip some CPU cycles (observably, RegExps take a bit more time, so it worth it), it also doesn't duplicate styles in 2 stylesheets inserted at different stages, especially good for Non-Chromium browsers, since allows to emulate less in case of Shadow DOM polyfill.

I do not really care about Shady DOM, so since I've encountered 2 tests failing with this patch under Shady DOM changes only affect Shadow DOM (either native or polyfilled, both are fine).

P.S. I'm experiencing significant performance penalty with multiple CSS mixins/vars in elements that have dozens of instances on page (especially under non-Chromium, this is one of results of exploration how to optimize something in styling system without crashing everything.

…d styles injections (once skipping CSS mixins and variables and second time with them)
nazar-pc added a commit to nazar-pc/CleverStyle-Framework that referenced this pull request Feb 1, 2016
@kevinpschaaf kevinpschaaf added the p0 label Feb 4, 2016
@kevinpschaaf
Copy link
Member

@sorvell PTAL

@samccone
Copy link
Contributor

samccone commented Feb 4, 2016

@nazar-pc do you have any perf metrics / test repo steps to back this change up?

@nazar-pc
Copy link
Contributor Author

nazar-pc commented Feb 4, 2016

I've tested performance using my codebase with many elements on page through Dev Tools Timeline. Unfortunately it is not easy to execute that code standalone.
It produces the same results, just works faster, thus no new tests, sorry.
This change have especially positive impact when using full polyfill.

@sorvell
Copy link
Contributor

sorvell commented Feb 13, 2016

Fixed via #3413. Thanks!

@sorvell sorvell closed this Feb 13, 2016
@nazar-pc nazar-pc deleted the perf-improvement-for-styles branch February 25, 2016 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants