Prevent performance regression in DEV due to warning arguments #7461
Conversation
This only affects Facebook website, not open source version of React. On the Facebook website, we don't have a transform for warnings and invariants. Therefore, expensive arguments will be calculated even if the warning doesn't fire. This fixes a few cases where that calculation might be more expensive than usually. In my testing, this brings down average row click time in Power Editor from ~300ms to ~220ms in __DEV__ (vs ~40ms in prod).
ghost
added
| @@ -69,7 +69,7 @@ if (__DEV__) { | |||
|
|
|||
| if (standardName != null) { | |||
| warning( | |||
| standardName == null, | |||
| false, | |||
There was a problem hiding this comment.
These are just consistency changes. The original code is kinda odd with two checks.
gaearon
changed the title
|
It is probably better to fix this right in fbjs: https://github.com/facebook/fbjs/blob/21f1f725e00de4c70f60b45f9db641b2a98e149c/packages/babel-preset-fbjs/plugins/dev-expression.js#L115 But this wouldn’t help us internally. |
|
👍 |
* Prevent internal performance regression This only affects Facebook website, not open source version of React. On the Facebook website, we don't have a transform for warnings and invariants. Therefore, expensive arguments will be calculated even if the warning doesn't fire. This fixes a few cases where that calculation might be more expensive than usually. In my testing, this brings down average row click time in Power Editor from ~300ms to ~220ms in __DEV__ (vs ~40ms in prod). * Put warning() that shows up in profile behind condition (cherry picked from commit 178cb7d)
|
I think we have an |
|
Or is that what you were referring to in your comment about dev-expression.js? |
|
Yea, dev-expression does it for invariant already & we could do it for warning too, but we don't do that transform internally so we'd still paying the cost on www (not in RN though as that uses the version of React we compile here). |
|
@gaearon I just updated to 15.3.1 and saw a great performance improvement (my incremental build time reduced from ~10 sec to ~4 sec.) I traced it and read the change log and saw that you did a lots of commits to improve performance. I just wanna thank you for your efforts and really appreciate that (and I'm sure as you know I'm just one out of many who appreciates it.) It really made life easier for me as waiting ~4 sec for each change is much much better than ~10 sec :-) Sorry if it's not the appropriate place for saying thanks but couldn't find better place for saying so. |
|
Interesting, this is actually not related and was probably caused by something else. |
|
I see, that's still valuable and I'm still thankful, though I wish I could find the one who reduced my wasted time waiting for compilation by 60% and thank him but I guess it takes so much time (Going through repositories of libraries I use and check them commit by commit and find out which commit did the magic!). Maybe we should improve git-bisect and develop an application called "Find the one you want to thank" for this purpose 😃 |
Expensive arguments shouldn’t be calculated if the warning doesn't fire.
This fixes a few cases where that calculation might be more expensive than usually.
In my testing, this brings down average row click time in Power Editor from ~300ms to ~220ms in
__DEV__(vs ~40ms in prod).