-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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 JS performance by storing length before comparing to it in loops #80515
Conversation
Some changes occurred in HTML/CSS/JS. |
More js cleanup Part of rust-lang#79052 (Same kind as rust-lang#80515). This one is about some small fixes: * Replacing some loops with `onEachLazy`. * Removing unused function arguments. * Turn `buildHelperPopup` into a variable so it can be "replaced" once the function has been called once so it's not called again. r? `@jyn514`
☔ The latest upstream changes (presumably #80554) made this pull request unmergeable. Please resolve the merge conflicts. |
8819166
to
9f21834
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the new code isn't any easier to read, and it creates a lot of churn.
9f21834
to
a59b765
Compare
Updated! |
a59b765
to
e727600
Compare
@Nemo157 Updated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rest of the changes all look correct 👍
@@ -1710,7 +1706,7 @@ function defocusSearchBar() { | |||
if (smallest === null) { | |||
break; | |||
} | |||
for (x = 0; x < arrays.length && ret.length < MAX_RESULTS; ++x) { | |||
for (x = 0; x < arrays_len && ret.length < MAX_RESULTS; ++x) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is using the non-lexical variable from line 1700, right? JS is fun 😭.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It's a syntax nightmare. ^^'
📌 Commit e727600 has been approved by |
…laumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#77693 (Add test for rust-lang#59352) - rust-lang#80515 (Improve JS performance by storing length before comparing to it in loops) - rust-lang#81030 (Update mdbook) - rust-lang#81033 (Remove useless `clean::Variant` struct) - rust-lang#81049 (inline: Round word-size cost estimates up) - rust-lang#81054 (Drop a few unneeded borrows) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Since #79052 is quite complicated to review, I suggested to split into smaller parts. This first part is mostly about saving the array length into a variable (I tried to not change anything else as much as possible 😃 ).
r? @jyn514