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: Eliminate redundant call to git.describedVersion #248

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

adpi2
Copy link
Member

@adpi2 adpi2 commented Sep 4, 2024

While I was profiling the loading time of softwaremill/sttp build, I noticed that sbt-git takes about half of the CPU time. Indeed it seems that git.describedVersion is quite expensive as it needs to unpack Git object directories. The problem is this operation is repeated too many times: once for each sub-project of the build (~200 projects in the case of sttp).

This PR eliminates the redundant calls to describedVersion: if the input settings of the ThisProject and ThisBuild are the same, the project reuses ThisBuild / gitDescribedVersion instead of recomputing it.

On softwaremill/sttp it reduces the loading time by 45% (12 seconds instead of 22).

@adpi2
Copy link
Member Author

adpi2 commented Sep 5, 2024

Before

sbt-git-cpu

After

sbt-git-cpu-after

@adpi2
Copy link
Member Author

adpi2 commented Sep 7, 2024

@SethTisue can you review this?

Copy link
Member

@SethTisue SethTisue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great catch!!

once this is merged let's publish a new version and pull it in downstream in sbt-ci-release, which is how many of the projects I work on get their sbt-git

@mikegpl
Copy link

mikegpl commented Sep 18, 2024

Hello, this looks promising in projects I've been working with too - thank you @adpi2 for your contribution and @SethTisue for your quick review ;) I just wanted to kindly follow up and check if there's a timeline for merging and releasing version with these changes.

@SethTisue SethTisue merged commit 76cb0e4 into sbt:main Sep 18, 2024
1 check passed
@SethTisue
Copy link
Member

re: publishing, I've opened #253

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.

3 participants