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.

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

Cache file contents in the session(s) #364

Merged
merged 8 commits into from Aug 27, 2015
Merged

Cache file contents in the session(s) #364

merged 8 commits into from Aug 27, 2015

Conversation

birkenfeld
Copy link
Collaborator

This series of commits makes an attempt to avoid re-reading files from disk over and over. It also makes all Session access use a Rc pointer, so that the session isn't cloned for every match.

This gives a large speedup in debug mode (doing racer complete std::io:: drops from 1.14s to 0.48s on my machine), probably because the BufReader implementation is really slow when unoptimized.

In a release build I get a ~25% speedup from 30ms to 22ms using kbknapp's method from here: https://gist.github.com/kbknapp/fb28a49d7a0fe8b8e400 - allocations are down from from 82k (19M bytes) to 56k (7M bytes).

In daemon mode every request creates its own root Session, so files are not cached. This is probably a good thing as long as the cache is never invalidated e.g. based on mtime.

@birkenfeld
Copy link
Collaborator Author

(updated benchmark numbers in description)

@dleslie
Copy link

dleslie commented Aug 26, 2015

As a person who codes on a slower machine, allow me to say thank-you.

On Wed, Aug 26, 2015 at 10:18 AM, Georg Brandl notifications@github.com
wrote:

(updated benchmark numbers in description)


Reply to this email directly or view it on GitHub
#364 (comment).

phildawes added a commit that referenced this pull request Aug 27, 2015
Cache file contents in the session(s)
@phildawes phildawes merged commit 563402c into racer-rust:master Aug 27, 2015
@phildawes
Copy link
Collaborator

Awesome, thanks @birkenfeld! Have merged.

BTW, I might be wrong but I don't think the session needs to be returned in the match object, I think it was just convenience when @damienrg added the Session to handle temporary file translation.

If instead we passed the session as a parameter to all calls then we wouldn't need RC/RefCell because mutable borrows would suffice. Does that sound correct to you?

Cheers!

@birkenfeld
Copy link
Collaborator Author

@phildawes so there is basically no reason to have multiple Session objects? If so, that can simplify the matter a lot.

@birkenfeld birkenfeld deleted the file_cache branch August 27, 2015 12:27
@birkenfeld
Copy link
Collaborator Author

We'll likely still need the RefCell for the file cache, since using a mutable borrow is going to be very unpleasant - many of the functions make liberal use of multiple borrowing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants