Skip to content

Conversation

@nnethercote
Copy link
Contributor

We currently heap-allocate each one twice, once for Interner::names and
once for Interner::strings. Using Rc instead means each one is allocated
once and then shared.

This speeds up numerous rustc-perf runs, the best by 4%.

Here are details for the ones with an improvement of 1% or more:

coercions-check
        avg: -2.3%      min: -4.1%      max: -0.8%
coercions
        avg: -1.3%      min: -2.8%      max: -0.8%
coercions-opt
        avg: -1.5%      min: -2.7%      max: -0.6%
tuple-stress
        avg: -0.5%      min: -1.3%      max: 0.1%
tuple-stress-check
        avg: -0.6%      min: -1.3%      max: -0.3%
tuple-stress-opt
        avg: -0.7%      min: -1.3%      max: -0.4%

@rust-highfive
Copy link
Contributor

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

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

We currently heap-allocate each one twice

:D

@Zoxc, do you see a problem with using Rc here? The ref-count should never be changed from different threads, as far as I can tell.

@Zoxc
Copy link
Contributor

Zoxc commented May 9, 2018

This use of Rc is fine. It will need an unsafe impl Send for Interner {} though. Add a note that the impl is safe because the ref count is only modified at one thread at a time. It is modified during insertion and during destruction. These may happen on different threads, but are mutually excluded.

@michaelwoerister
Copy link
Member

OTOH, there wouldn't be a real downside to using Lrc, right?

@Zoxc
Copy link
Contributor

Zoxc commented May 9, 2018

It would be slower.

@michaelwoerister
Copy link
Member

Yes, but I wouldn't expect the difference to be noticeable.

We currently heap-allocate each one twice, once for Interner::names and
once for Interner::strings. Using Rc instead means each one is allocated
once and then shared.

This speeds up numerous rustc-perf runs, the best by 4%.
@nnethercote
Copy link
Contributor Author

Updated with the unsafe impl Send.

r? @Zoxc

@rust-highfive rust-highfive assigned Zoxc and unassigned aturon May 9, 2018
bors added a commit that referenced this pull request May 10, 2018
Allocate Symbol strings from an arena

This is an alternative to #50549

cc @nnethercote

r? @michaelwoerister
@nnethercote
Copy link
Contributor Author

#50607 is a much better way of doing things.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 12, 2018
Allocate Symbol strings from an arena

This is an alternative to rust-lang#50549

cc @nnethercote

r? @michaelwoerister
@nnethercote nnethercote deleted the Rc-interner branch December 12, 2018 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants