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.

Sign up for GitHub

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

Jump to bottom

Use raw_entry for more efficient interning #56324

Merged
merged 2 commits into from
Nov 30, 2018
Merged

Use raw_entry for more efficient interning #56324

merged 2 commits into from
Nov 30, 2018

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Nov 28, 2018

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 28, 2018
@eddyb
Copy link
Member

eddyb commented Nov 28, 2018

LGTM. r? @nikomatsakis

@bors try (for perf)

@rust-highfive rust-highfive assigned nikomatsakis and unassigned eddyb Nov 28, 2018
@bors
Copy link
Contributor

bors commented Nov 28, 2018

⌛ Trying commit e1adef66a8383d29a92ec9069b78d12c4aa68245 with merge fb4874d830c2e1e085a3bb8a83ee9a54b7570c31...

@bors
Copy link
Contributor

bors commented Nov 28, 2018

☀️ Test successful - status-travis
State: approved= try=True

@Zoxc
Copy link
Contributor Author

Zoxc commented Nov 28, 2018

@rust-timer build fb4874d

@rust-timer
Copy link
Collaborator

Insufficient permissions to issue commands to rust-timer.

@Mark-Simulacrum
Copy link
Member

@rust-timer build fb4874d830c2e1e085a3bb8a83ee9a54b7570c31

@rust-timer
Copy link
Collaborator

Success: Queued fb4874d830c2e1e085a3bb8a83ee9a54b7570c31 with parent b68fc18, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit fb4874d830c2e1e085a3bb8a83ee9a54b7570c31

@eddyb
Copy link
Member

eddyb commented Nov 29, 2018

@gankro @Amanieu this looks bad, huh.

@Zoxc
Copy link
Contributor Author

Zoxc commented Nov 29, 2018

Well that is odd. Let's see if performance is unchanged if I skip raw_entry and do a double lookup.

@bors try

@bors
Copy link
Contributor

bors commented Nov 29, 2018

⌛ Trying commit 07d10fe3b2e8061212c81d789f65449ae0070132 with merge 696602f085fc42a2c0560b36627e6eeda1374c81...

@Amanieu
Copy link
Member

Amanieu commented Nov 29, 2018

I've seen this before in hashbrown. The solution is to inline all the things.

@bors
Copy link
Contributor

bors commented Nov 29, 2018

☀️ Test successful - status-travis
State: approved= try=True

@Mark-Simulacrum
Copy link
Member

@rust-timer build 696602f085fc42a2c0560b36627e6eeda1374c81

@rust-timer
Copy link
Collaborator

Success: Queued 696602f085fc42a2c0560b36627e6eeda1374c81 with parent a49316d, comparison URL.

@Zoxc
Copy link
Contributor Author

Zoxc commented Nov 29, 2018

I've added inline attributes to things that weren't inlined.

@bors try

@bors
Copy link
Contributor

bors commented Nov 29, 2018

⌛ Trying commit 74e4e9c1d0801dec1305f675bcb660be1b68cfb6 with merge b4c80f1bb327a736dce362d4a059b1d414b5182c...

@bors
Copy link
Contributor

bors commented Nov 29, 2018

☀️ Test successful - status-travis
State: approved= try=True

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 696602f085fc42a2c0560b36627e6eeda1374c81

@Zoxc
Copy link
Contributor Author

Zoxc commented Nov 29, 2018

@bors try

@bors
Copy link
Contributor

bors commented Nov 29, 2018

⌛ Trying commit 9afd6877ae657b194450a772fd27325fe4542133 with merge 4be721be11349e95dc58f71db4148a60eb5439c2...

@Zoxc
Copy link
Contributor Author

Zoxc commented Nov 29, 2018

I see a 5% speedup in item-bodies checking locally now.

@nikomatsakis
Copy link
Contributor

r=me pending perf results

@bors
Copy link
Contributor

bors commented Nov 29, 2018

☀️ Test successful - status-travis
State: approved= try=True

@Zoxc
Copy link
Contributor Author

Zoxc commented Nov 29, 2018

@rust-timer build 4be721be11349e95dc58f71db4148a60eb5439c2

@rust-timer
Copy link
Collaborator

Insufficient permissions to issue commands to rust-timer.

@michaelwoerister
Copy link
Member

@rust-timer build 4be721be11349e95dc58f71db4148a60eb5439c2

@rust-timer
Copy link
Collaborator

Success: Queued 4be721be11349e95dc58f71db4148a60eb5439c2 with parent 0c1dc62, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 4be721be11349e95dc58f71db4148a60eb5439c2

@Zoxc
Copy link
Contributor Author

Zoxc commented Nov 30, 2018

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Nov 30, 2018

📌 Commit 946ea14 has been approved by nikomatsakis

@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 Nov 30, 2018
@michaelwoerister
Copy link
Member

That's pretty awesome :)
cc @rust-lang/wg-compiler-performance

kennytm added a commit to kennytm/rust that referenced this pull request Nov 30, 2018
@kennytm
Rollup merge of rust-lang#56324 - Zoxc:int-ext, r=nikomatsakis
ecfe721 
Use raw_entry for more efficient interning

Fixes rust-lang#56308 (comment)
@kennytm kennytm mentioned this pull request Nov 30, 2018
Merged
bors added a commit that referenced this pull request Nov 30, 2018
@bors
Auto merge of #56381 - kennytm:rollup, r=kennytm
d09466c 
Rollup of 19 pull requests

Successful merges:

 - #55011 (Add libstd Cargo feature "panic_immediate_abort")
 - #55821 (Use sort_by_cached_key when the key function is not trivial/free)
 - #56014 (add test for issue #21335)
 - #56131 (Assorted tweaks)
 - #56214 (Implement chalk unification routines)
 - #56216 (Add TryFrom<&[T]> for [T; $N] where T: Copy)
 - #56268 (Reuse the `P` in `InvocationCollector::fold_{,opt_}expr`.)
 - #56324 (Use raw_entry for more efficient interning)
 - #56336 (Clean up and streamline the pretty-printer)
 - #56337 (Fix const_fn ICE with non-const function pointer)
 - #56339 (Remove not used option)
 - #56341 (Rename conversion util; remove duplicate util in librustc_codegen_llvm.)
 - #56349 (rustc 1.30.0's linker flavor inference is a non-backwards compat change to -Clinker)
 - #56355 (Add inline attributes and add unit to CommonTypes)
 - #56360 (Optimize local linkchecker program)
 - #56364 (Fix panic with outlives in existential type)
 - #56365 (Stabilize self_struct_ctor feature.)
 - #56367 (Moved some feature gate tests to correct location)
 - #56373 (Update books)
@bors
Copy link
Contributor

bors commented Nov 30, 2018

☔ The latest upstream changes (presumably #56381) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 30, 2018
@bors bors merged commit 946ea14 into rust-lang:master Nov 30, 2018
@Zoxc Zoxc deleted the int-ext branch December 1, 2018 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@Zoxc @eddyb @bors @rust-timer @Mark-Simulacrum @Amanieu @nikomatsakis @michaelwoerister @rust-highfive