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

Introduce and use LrcRef in ImplicitCtxt #57029

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@Zoxc
Copy link
Contributor

Zoxc commented Dec 21, 2018

LrcRef is basically just &Lrc without the double indirection, but still retains the ability to clone the Lrc.

r? @michaelwoerister

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Dec 21, 2018

I'd like to see a measurable performance win before I review this. It seems easy to get wrong and the code violates the preconditions given in from_raw's documentation.

@bors try

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 21, 2018

⌛️ Trying commit fe9b198 with merge 61f0d09...

bors added a commit that referenced this pull request Dec 21, 2018

Auto merge of #57029 - Zoxc:query-perf6, r=<try>
Introduce and use LrcRef in ImplicitCtxt

`LrcRef` is basically just `&Lrc` without the double indirection, but still retains the ability to clone the `Lrc`.

r? @michaelwoerister
@Zoxc

This comment has been minimized.

Copy link
Contributor

Zoxc commented Dec 21, 2018

This is unlikely to give a measurable improvement, it just avoids cloning the Lrc. We can fix the preconditions on frow_raw. cc @rust-lang/libs

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Dec 21, 2018

If it isn't a performance win, then why add unsafe code?

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 21, 2018

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

@Zoxc

This comment has been minimized.

Copy link
Contributor

Zoxc commented Dec 22, 2018

@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Dec 22, 2018

Success: Queued 61f0d09 with parent 6d34ec1, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Dec 22, 2018

Finished benchmarking try commit 61f0d09

@Dylan-DPC

This comment has been minimized.

Copy link
Member

Dylan-DPC commented Jan 21, 2019

ping from triage @Zoxc you have few conflicts to address

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