Refactor StyleSheet.create to improve performance #299
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Removing this abstraction that was only used in one place simplifies the code a little and theoretically should improve performance by avoiding some extra function calls and object creation. I say theoretically because my rudimentary profiling is pretty inconclusive. It is possible that my benchmark scenario is non-representative, the browser is optimizing this away anyway. In either case, this seems like a small improvement.
Instead of checking this on every iteration, we can just set it when the module first runs, and then switch it out when minify() is called. This should keep the hot path as fast as possible.
I forgot to do this in my rollup PR.
|
FYI @Soreine I ended up refactoring your minify check a little here. Care to review this PR? |
Soreine
approved these changes
Feb 16, 2018
There was a problem hiding this comment.
It looks better indeed.
When, because of performance issues, some piece of code is unnatural or surprising, but it should not be changed without thought, it can be worth adding a comment.
I personally use this format: // PERF: Reasons why the code is that way...
|
Thanks for the suggestion! I thought it seemed fairly natural, but I guess it doesn't hurt to drop in a few comments. I'll follow up with that. |
|
Hmm yes, I wasn't clear, in this case the code is natural, it's just not
obvious that it's a hot code path :)
Le 16 févr. 2018 18:11, "Joe Lencioni" <notifications@github.com> a écrit :
… Merged #299 <#299>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#299 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AFYimawcsxEAevIJRI2QcQbAQGqojCQaks5tVbbbgaJpZM4SHxR5>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims to improve performance of this hot path by reducing the number of function calls and object creation.