Need to rebase more than one path per once #8

Closed
ttim opened this Issue Nov 9, 2015 · 2 comments

Comments

Projects
None yet
2 participants
@ttim

ttim commented Nov 9, 2015

Pants will rebase two paths instead of previous one: build root path and work directory path. So we need some API to express this intention in zincutils. Currently pants executes rebase_from_path twice which can be inefficiently.

@peiyuwang

This comment has been minimized.

Show comment
Hide comment
@peiyuwang

peiyuwang Nov 3, 2016

https://rbcommons.com/s/twitter/r/4352/ is to address this (note in pants repo because zincutils has been merged into pants repo)

https://rbcommons.com/s/twitter/r/4352/ is to address this (note in pants repo because zincutils has been merged into pants repo)

peiyuwang added a commit to pantsbuild/pants that referenced this issue Nov 9, 2016

Perf improvement: rebase analyis file once instead of multiple times
We have this outstanding issue pantsbuild/zincutils#8 to scan analysis file once, currently it does multiple scans, one per replaced pairs.

In #3962 new zinc analysis files are bigger, almost doubled in time in local testing. Therefore optimization becomes necessary.

From profiling, each of the following accounts for about 1/3 of the rebase total time:

- file write
- read() to iterate through lines
- everything else (string replacement)

This PR cuts the time from the first two items by a factor of two since there are two scans. That nets 1/3 saving

Testing Done:
https://travis-ci.org/peiyuwang/pants/builds/173091425
https://travis-ci.org/peiyuwang/pants/builds/174378023

Bugs closed: 4026

Reviewed at https://rbcommons.com/s/twitter/r/4352/
@peiyuwang

This comment has been minimized.

Show comment
Hide comment
@peiyuwang

peiyuwang Nov 9, 2016

Reviewed at https://rbcommons.com/s/twitter/r/4352/

Committed as 04afcc8476afd5bbff6326485bfb1f66b39b3d72

Reviewed at https://rbcommons.com/s/twitter/r/4352/

Committed as 04afcc8476afd5bbff6326485bfb1f66b39b3d72

@peiyuwang peiyuwang closed this Nov 9, 2016

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