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

Allocate Symbol strings from an arena #50607

Merged
merged 1 commit into from
May 12, 2018
Merged

Allocate Symbol strings from an arena #50607

merged 1 commit into from
May 12, 2018

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented May 10, 2018

This is an alternative to #50549

cc @nnethercote

r? @michaelwoerister

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

@bors try

@bors
Copy link
Contributor

bors commented May 10, 2018

⌛ Trying commit ca32340 with merge 40780cc...

bors added a commit that referenced this pull request May 10, 2018
@bors
Auto merge of #50607 - Zoxc:symbol-arena, r=<try>
40780cc 
Allocate Symbol strings from an arena

This is an alternative to #50549

cc @nnethercote

r? @michaelwoerister
@bors
Copy link
Contributor

bors commented May 10, 2018

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

@Zoxc
Copy link
Contributor Author

Zoxc commented May 10, 2018

@Mark-Simulacrum I'd like a perf run here

@kennytm kennytm added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 10, 2018
@nnethercote
Copy link
Contributor

Nitpick: All the comments in this patch would benefit from a full stop ('.') at the end :)

@nnethercote
Copy link
Contributor

I did a local benchmarking run. The results were excellent, and much better than those from #50549.

helloworld-check
	avg: -4.9%	min: -5.2%	max: -4.7%
unify-linearly-check
	avg: -3.9%	min: -4.6%	max: -3.1%
issue-46449-check
	avg: -3.8%	min: -4.3%	max: -3.2%
parser-check
	avg: -3.5%	min: -4.2%	max: -2.5%
deeply-nested-check
	avg: -3.0%	min: -3.9%	max: -2.3%
issue-46449
	avg: -1.8%	min: -3.7%	max: -1.4%
deeply-nested-opt
	avg: -1.9%	min: -3.6%	max: -0.9%
issue-46449-opt
	avg: -1.2%	min: -3.5%	max: -0.5%
deeply-nested
	avg: -2.1%	min: -3.5%	max: -1.2%
regression-31157-check
	avg: -2.0%	min: -2.8%	max: -1.2%
futures-check
	avg: -1.7%	min: -2.6%	max: -0.6%
tokio-webpush-simple-check
	avg: -1.7%	min: -2.5%	max: -1.0%
futures-opt
	avg: -1.4%	min: -2.4%	max: -0.4%
futures
	avg: -1.5%	min: -2.4%	max: -0.5%
regex-check
	avg: -1.9%	min: -2.4%	max: -0.9%
coercions-check
	avg: -1.1%	min: -2.0%	max: -0.4%
regex-opt
	avg: -0.9%	min: -2.0%	max: -0.1%
regex
	avg: -1.0%	min: -1.9%	max: -0.2%
regression-31157-opt
	avg: -0.2%	min: -1.8%	max: 1.7%
hyper-check
	avg: -1.2%	min: -1.7%	max: -0.6%
parser-opt
	avg: -1.4%	min: -1.7%	max: -1.2%
encoding-check
	avg: -1.2%	min: -1.7%	max: -0.6%
html5ever-check
	avg: -0.7%	min: -1.6%	max: -0.1%
regression-31157
	avg: -1.0%	min: -1.6%	max: -0.4%
helloworld
	avg: -1.5%	min: -1.6%	max: -1.4%
parser
	avg: -1.4%	min: -1.6%	max: -1.1%
unify-linearly
	avg: -1.4%	min: -1.6%	max: -1.2%
encoding-opt
	avg: -0.8%	min: -1.5%	max: -0.0%
coercions-opt
	avg: -0.8%	min: -1.5%	max: -0.3%
hyper-opt
	avg: -0.8%	min: -1.5%	max: -0.2%
helloworld-opt
	avg: -1.4%	min: -1.5%	max: -1.3%
unify-linearly-opt
	avg: -1.3%	min: -1.5%	max: -1.2%
coercions
	avg: -0.7%	min: -1.5%	max: -0.1%
hyper
	avg: -0.8%	min: -1.5%	max: -0.3%
encoding
	avg: -0.6%	min: -1.4%	max: 0.3%
unused-warnings-check
	avg: -1.1%	min: -1.4%	max: -1.0%
html5ever
	avg: -0.8%	min: -1.4%	max: -0.4%
tuple-stress-opt
	avg: -0.7%	min: -1.3%	max: -0.4%
tuple-stress-check
	avg: -0.6%	min: -1.2%	max: -0.2%
unused-warnings
	avg: -1.0%	min: -1.1%	max: -0.9%
syn-check
	avg: -0.7%	min: -1.1%	max: -0.3%
unused-warnings-opt
	avg: -0.9%	min: -1.0%	max: -0.8%
syn
	avg: -0.5%	min: -1.0%	max: -0.2%
syn-opt
	avg: -0.3%	min: -1.0%	max: 0.2%

Nice work, @Zoxc!

@nnethercote nnethercote mentioned this pull request May 11, 2018
Closed
@michaelwoerister
Copy link
Member

Let's do it :)

@bors r+

@bors
Copy link
Contributor

bors commented May 11, 2018

📌 Commit ca32340 has been approved by michaelwoerister

@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 May 11, 2018
@michaelwoerister
Copy link
Member

Here's the complementary perf link: http://perf.rust-lang.org/compare.html?start=57dc98434eb818733dbc941405fdee59e5c3a023&end=40780cc0b23270ffb2a8554581ded2c6b6b02ed9&stat=instructions%3Au

@Mark-Simulacrum
Copy link
Member

@bors rollup

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 12, 2018
@Mark-Simulacrum
Rollup merge of rust-lang#50607 - Zoxc:symbol-arena, r=michaelwoerister
575ac94 
Allocate Symbol strings from an arena

This is an alternative to rust-lang#50549

cc @nnethercote

r? @michaelwoerister
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request May 12, 2018
Merged
bors added a commit that referenced this pull request May 12, 2018
@bors
Auto merge of #50686 - Mark-Simulacrum:rollup, r=Mark-Simulacrum
ff2ac35 
Rollup of 13 pull requests

Successful merges:

 - #50544 (Cleanup some dependencies)
 - #50545 (Made some functions in time module const)
 - #50550 (use fmt::Result where applicable)
 - #50558 (Remove all reference to DepGraph::work_products)
 - #50602 (Update canonicalize docs)
 - #50607 (Allocate Symbol strings from an arena)
 - #50613 (Migrate the toolstate update bot to rust-highfive)
 - #50624 (fs::write: Add example writing a &str)
 - #50634 (Do not silently truncate offsets for `read_at`/`write_at` on emscripten)
 - #50644 (AppVeyor: Read back trace from crash dump on failure.)
 - #50661 (Ignore non .rs files for tidy libcoretest)
 - #50663 (rustc: Allow an edition's feature on that edition)
 - #50667 (rustc: Only suggest deleting `extern crate` if it works)

Failed merges:
@bors bors merged commit ca32340 into rust-lang:master May 12, 2018
@Zoxc Zoxc deleted the symbol-arena branch April 3, 2019 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@michaelwoerister michaelwoerister

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-perf Status: Waiting on a perf run to be completed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Zoxc @michaelwoerister @bors @nnethercote @Mark-Simulacrum @kennytm @rust-highfive