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

Avoid allocations in `Decoder::read_str`. #37064

Merged
merged 1 commit into from Oct 13, 2016

Conversation

Projects
None yet
5 participants
@nnethercote
Contributor

nnethercote commented Oct 9, 2016

opaque::Decoder::read_str is very hot within rustc due to its use in
the reading of crate metadata, and it currently returns a String. This
commit changes it to instead return a Cow<str>, which avoids a heap
allocation.

This change reduces the number of calls to malloc by almost 10% in
some benchmarks.

This is a [breaking-change] to libserialize.

@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive Oct 9, 2016

Collaborator

r? @erickt

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

Collaborator

rust-highfive commented Oct 9, 2016

r? @erickt

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

@nnethercote

This comment has been minimized.

Show comment
Hide comment
@nnethercote

nnethercote Oct 9, 2016

Contributor

r? @eddyb

Contributor

nnethercote commented Oct 9, 2016

r? @eddyb

@nnethercote

This comment has been minimized.

Show comment
Hide comment
@nnethercote

nnethercote Oct 9, 2016

Contributor

(This is a carry-over of #37022 because I borked my git clone.)

Contributor

nnethercote commented Oct 9, 2016

(This is a carry-over of #37022 because I borked my git clone.)

@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb

eddyb Oct 9, 2016

Member

@bors r+

Member

eddyb commented Oct 9, 2016

@bors r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 9, 2016

Contributor

📌 Commit d6117f8 has been approved by eddyb

Contributor

bors commented Oct 9, 2016

📌 Commit d6117f8 has been approved by eddyb

Avoid allocations in `Decoder::read_str`.
`opaque::Decoder::read_str` is very hot within `rustc` due to its use in
the reading of crate metadata, and it currently returns a `String`. This
commit changes it to instead return a `Cow<str>`, which avoids a heap
allocation.

This change reduces the number of calls to `malloc` by almost 10% in
some benchmarks.

This is a [breaking-change] to libserialize.
@nnethercote

This comment has been minimized.

Show comment
Hide comment
@nnethercote

nnethercote Oct 9, 2016

Contributor

@eddyb: bleh, I pushed with some uncommitted changes. New version should be better.

Contributor

nnethercote commented Oct 9, 2016

@eddyb: bleh, I pushed with some uncommitted changes. New version should be better.

@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb

eddyb Oct 9, 2016

Member

@bors r+

Member

eddyb commented Oct 9, 2016

@bors r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 9, 2016

Contributor

📌 Commit b043e11 has been approved by eddyb

Contributor

bors commented Oct 9, 2016

📌 Commit b043e11 has been approved by eddyb

jonathandturner added a commit to jonathandturner/rust that referenced this pull request Oct 11, 2016

Rollup merge of #37064 - nnethercote:read_str, r=eddyb
Avoid allocations in `Decoder::read_str`.

`opaque::Decoder::read_str` is very hot within `rustc` due to its use in
the reading of crate metadata, and it currently returns a `String`. This
commit changes it to instead return a `Cow<str>`, which avoids a heap
allocation.

This change reduces the number of calls to `malloc` by almost 10% in
some benchmarks.

This is a [breaking-change] to libserialize.

bors added a commit that referenced this pull request Oct 12, 2016

Auto merge of #37093 - jonathandturner:rollup, r=jonathandturner
Rollup of 10 pull requests

- Successful merges: #36692, #36743, #36762, #36991, #37023, #37050, #37056, #37064, #37066, #37067
- Failed merges:

bors added a commit that referenced this pull request Oct 12, 2016

Auto merge of #37093 - jonathandturner:rollup, r=jonathandturner
Rollup of 10 pull requests

- Successful merges: #36692, #36743, #36762, #36991, #37023, #37050, #37056, #37064, #37066, #37067
- Failed merges:

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Oct 12, 2016

Rollup merge of #37064 - nnethercote:read_str, r=eddyb
Avoid allocations in `Decoder::read_str`.

`opaque::Decoder::read_str` is very hot within `rustc` due to its use in
the reading of crate metadata, and it currently returns a `String`. This
commit changes it to instead return a `Cow<str>`, which avoids a heap
allocation.

This change reduces the number of calls to `malloc` by almost 10% in
some benchmarks.

This is a [breaking-change] to libserialize.

bors added a commit that referenced this pull request Oct 12, 2016

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Oct 12, 2016

Rollup merge of #37064 - nnethercote:read_str, r=eddyb
Avoid allocations in `Decoder::read_str`.

`opaque::Decoder::read_str` is very hot within `rustc` due to its use in
the reading of crate metadata, and it currently returns a `String`. This
commit changes it to instead return a `Cow<str>`, which avoids a heap
allocation.

This change reduces the number of calls to `malloc` by almost 10% in
some benchmarks.

This is a [breaking-change] to libserialize.

bors added a commit that referenced this pull request Oct 12, 2016

@bors bors merged commit b043e11 into rust-lang:master Oct 13, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nnethercote nnethercote deleted the nnethercote:read_str branch Oct 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment