Skip to content

Conversation

@notriddle
Copy link
Contributor

This might have made sense if the algorithm could use searchWords to skip having to look at searchIndex, but since it always does a substring check on both the stock word and the normalizedName, it doesn't seem to help performance anyway.

Profile: http://notriddle.com/rustdoc-html-demo-8/searchwords/index.html

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 15, 2023
@rustbot
Copy link
Collaborator

rustbot commented Dec 15, 2023

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

@notriddle notriddle force-pushed the notriddle/searchwords branch from cbf0365 to 44b2c32 Compare December 15, 2023 21:36
@rust-log-analyzer

This comment has been minimized.

This might have made sense if the algorithm could use `searchWords`
to skip having to look at `searchIndex`, but since it always
does a substring check on both the stock word and the normalizedName,
it doesn't seem to help performance anyway.
@notriddle notriddle force-pushed the notriddle/searchwords branch from 44b2c32 to 6b69ebc Compare December 15, 2023 23:26
@GuillaumeGomez
Copy link
Member

Looks good to me, thanks! Just to confirm: is it normal that it seems that the build time is taking longer or is it just some noise? If it's noise, r=me.

@notriddle
Copy link
Contributor Author

I didn’t change the Rust code at all, and the Build part measures Cargo, not JS at all.

@GuillaumeGomez
Copy link
Member

Oh. That's a huge misunderstanding on my end then. Makes the results even better then!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 16, 2023

📌 Commit 6b69ebc has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 16, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 16, 2023
…=GuillaumeGomez

rustdoc-search: remove parallel searchWords array

This might have made sense if the algorithm could use `searchWords` to skip having to look at `searchIndex`, but since it always does a substring check on both the stock word and the normalizedName, it doesn't seem to help performance anyway.

Profile: http://notriddle.com/rustdoc-html-demo-8/searchwords/index.html
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 16, 2023
…iaskrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#118644 (Add test for Apple's `-weak_framework` linker argument)
 - rust-lang#118828 (Remove dead codes in rustc_codegen_gcc)
 - rust-lang#119001 (rustdoc-search: remove parallel searchWords array)

r? `@ghost`
`@rustbot` modify labels: rollup
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 16, 2023
…=GuillaumeGomez

rustdoc-search: remove parallel searchWords array

This might have made sense if the algorithm could use `searchWords` to skip having to look at `searchIndex`, but since it always does a substring check on both the stock word and the normalizedName, it doesn't seem to help performance anyway.

Profile: http://notriddle.com/rustdoc-html-demo-8/searchwords/index.html
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 16, 2023
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#118644 (Add test for Apple's `-weak_framework` linker argument)
 - rust-lang#118828 (Remove dead codes in rustc_codegen_gcc)
 - rust-lang#118830 (Add support for `--env` on `tracked_env::var`)
 - rust-lang#119001 (rustdoc-search: remove parallel searchWords array)
 - rust-lang#119020 (remove `hex` dependency in bootstrap)

r? `@ghost`
`@rustbot` modify labels: rollup
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 16, 2023
…=GuillaumeGomez

rustdoc-search: remove parallel searchWords array

This might have made sense if the algorithm could use `searchWords` to skip having to look at `searchIndex`, but since it always does a substring check on both the stock word and the normalizedName, it doesn't seem to help performance anyway.

Profile: http://notriddle.com/rustdoc-html-demo-8/searchwords/index.html
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 17, 2023
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#118828 (Remove dead codes in rustc_codegen_gcc)
 - rust-lang#118830 (Add support for `--env` on `tracked_env::var`)
 - rust-lang#119001 (rustdoc-search: remove parallel searchWords array)
 - rust-lang#119011 (coverage: Regression test for `assert!(!false)`)
 - rust-lang#119020 (remove `hex` dependency in bootstrap)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Collaborator

bors commented Dec 17, 2023

⌛ Testing commit 6b69ebc with merge 2f19122...

@bors
Copy link
Collaborator

bors commented Dec 17, 2023

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 2f19122 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 17, 2023
@bors bors merged commit 2f19122 into rust-lang:master Dec 17, 2023
@rustbot rustbot added this to the 1.76.0 milestone Dec 17, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2f19122): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.1% [2.1%, 2.1%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.5% [-3.3%, -1.7%] 2
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.0% [0.0%, 0.0%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Bootstrap: 674.824s -> 670.539s (-0.63%)
Artifact size: 312.45 MiB -> 312.41 MiB (-0.01%)

@notriddle notriddle deleted the notriddle/searchwords branch December 17, 2023 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants