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

perf: optimize hash for build #4067

Merged
merged 2 commits into from Apr 5, 2018

Conversation

Projects
None yet
3 participants
@eed3si9n
Copy link
Member

commented Apr 5, 2018

This addresses a hot path, which was discovered by @retronym using FlameGraph, by removing intermediate creation of byte Arrays.

filesModifiedBytes in particular was bad as it was going through all *.class files, each generating an Array. This replaces that with fileModifiedHash that accepts MessageDigest.

flamegraph-reload-baseline_svg

According to the flamegraph, evalCommon footprint reduced from 4.5% to 3.6%. See https://gist.github.com/eed3si9n/d68f3aa91be7bbe55b0c8cf77e37cf98.
Using time sbt reload reload reload exit, the median reduced from 41.450s to 39.467s.

perf: optimize hash for build
This hot path was discovered by retronym using FlameGraph.
This removes intermediate creation of Array.

`filesModifiedBytes` in particular was bad as it was going through all `*.class` files, each generating an Array. This replaces that with `fileModifiedHash` that accepts `MessageDigest`.

According to the flamegraph, evalCommon footprint reduced from 4.5% to 3.6%.
Using `time sbt reload reload reload exit`, the median reduced from 41.450s to 39.467s.

@eed3si9n eed3si9n requested a review from dwijnand Apr 5, 2018

@eed3si9n eed3si9n added the in progress label Apr 5, 2018

@eed3si9n

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2018

/cc @cunei

If we focus on the remaining 3.6%, it turns out that one of the hot paths is sbt.internal.io.MilliNative.getModifiedTime.

evalcommon

So I tried -Dsbt.io.jdktimestamps=true:

$ time sbt -Dsbt.io.jdktimestamps=true reload reload reload exit

This reduced the median from 39.467s to 37.275s (with minimum measurement 35.674s). Given that build timestamp is not subject to subsecond incremental compilation, non-milisec accuracy I think is worth considering for this use case.

@dwijnand
Copy link
Member

left a comment

lgtm.

I've wanted to land these changes since October..

@dwijnand dwijnand added this to the 1.1.3 milestone Apr 5, 2018

@eed3si9n eed3si9n merged commit b7d3959 into sbt:1.1.x Apr 5, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@eed3si9n eed3si9n deleted the eed3si9n:wip/reload-perf branch Apr 5, 2018

@eed3si9n eed3si9n removed the in progress label Apr 5, 2018

eatkins pushed a commit to eatkins/io that referenced this pull request Apr 11, 2018

@fommil

This comment has been minimized.

Copy link

commented Apr 11, 2018

you'll get even better timestamp performance if you use nio to walk a directory which means you get all timestamps of a directory in one OS call. I hope a change to fully nio is on the horizon for 2.0.

@dwijnand

This comment has been minimized.

Copy link
Member

commented Apr 11, 2018

good to know!

eatkins pushed a commit to eatkins/io that referenced this pull request Apr 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.