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

migrate region-maps to on-demand #41057

Closed
nikomatsakis opened this issue Apr 4, 2017 · 3 comments
Closed

migrate region-maps to on-demand #41057

nikomatsakis opened this issue Apr 4, 2017 · 3 comments
Labels
A-incr-comp Area: Incremental compilation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

As part of #40746, we need to migrate the region-maps into incremental. However, @cramertj's first effort (#40873) revealed that this is a bit more complicated than I thought at first. In particular the current regions are "whole-crate", but moving them to on-demand then introduces unacceptable dependency edges everywhere.

I think the overall structure that we want is that one does not request the "region maps" for the entire crate, but rather for a particular function or item (probably a body, though currently closures would share a region maps with their containing function).

This is made somewhat more complex by the fact that the RegionMaps construction code is quite old and a bit complex. It has for example a whole bunch of ref-cells that I think are primarily just an artifact of it being old -- i.e., the mutations should all be occuring during the construction phase, and not later.

Another complicating factor is the fact that we intern code extents to a small integer. Currently this interning is done globally (i.e., across all functions). I think it'd be fine, however, to do that locally (i.e., have a distinct vector for each function).

Each of the above complications can be refactored away -- probably! -- but it'll be non-trivial. It certainly makes sense to do this in phases. I'm not sure what those phases ought to be. Probably this:

  • Purge the refcells -- use &mut borrows during middle::region::resolve_crate() and otherwise the data should be immutable. If this fails, we should figure out why and refactor accordingly.
  • Once the refcells are gone, try to thread through an appropriate fn or body-id into every call, and restructure the data to be per-fn.
  • Once that is done, we can actually break the region map structure to be built "on demand" and do the construction per function.
@nikomatsakis nikomatsakis added A-incr-comp Area: Incremental compilation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 4, 2017
@nikomatsakis
Copy link
Contributor Author

@cramertj are you still interested in tackling this job?

@cramertj
Copy link
Member

cramertj commented Apr 4, 2017

Yes!

@nikomatsakis
Copy link
Contributor Author

This was done in #41662

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-incr-comp Area: Incremental compilation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants