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

make Target cacheable in the ProductGraph #3991

Closed
kwlzn opened this issue Oct 20, 2016 · 4 comments
Closed

make Target cacheable in the ProductGraph #3991

kwlzn opened this issue Oct 20, 2016 · 4 comments
Assignees

Comments

@kwlzn
Copy link
Member

kwlzn commented Oct 20, 2016

currently, BuildGraph construction is responsible for the ~majority of our runtime in the warm case. much of the cost here lies in Target instantiation which happens on each run by way of converting cached LegacyTarget/TargetAdaptor objects.

if we can instead construct Target objects directly in the daemon+engine context and thus make them cacheable in the resident ProductGraph, then we can theoretically skirt their construction costs on warm runs and simply link them together within a per-runBuildGraph.

in order to do this, we'll probably have to:

  1. kill all mutable state on Target
  2. remove Target->BuildGraph linkages
@kwlzn kwlzn added this to the v2 engine/daemon adoption milestone Oct 20, 2016
@kwlzn kwlzn self-assigned this Oct 27, 2016
@stuhood
Copy link
Member

stuhood commented Feb 17, 2017

If the warm performance is "good enough" for now, I suspect that focusing on #4213 and #3560 would be more holistic fixes. I'm going to drop this from the milestones.

@stuhood stuhood removed this from the v2 engine: daemon-by-default beta milestone Feb 17, 2017
@kwlzn
Copy link
Member Author

kwlzn commented Feb 17, 2017

sgtm.

@kwlzn kwlzn added this to the Daemon Post-Beta milestone Mar 1, 2018
@stuhood
Copy link
Member

stuhood commented Mar 15, 2018

It seems likely that this should be dropped entirely in favor of #4769, which proposes to not create a BuildGraph at all for frontend Task instances that have been updated to have product requirements. That would mean that those tasks would immediately begin consuming the v2 products that we're exposing, so they would need a lot more polish via #4535.

@stuhood
Copy link
Member

stuhood commented Mar 29, 2018

I'm going to close this. I believe that #4535 and #4769 invalidate it.

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

No branches or pull requests

2 participants