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

Reduce string allocations when working with paths. #5543

Merged
merged 3 commits into from
Jun 1, 2018
Merged

Reduce string allocations when working with paths. #5543

merged 3 commits into from
Jun 1, 2018

Conversation

tjni
Copy link
Contributor

@tjni tjni commented May 25, 2018

Issue: #5541

Context

This rewrites in two places the logic checking if one string path is the start of another so that it does not allocate new strings. I verified in a profile run of my build that the large amount of memory that I observed my build using is no longer present. Please see the attached issue for the original context.

Contributor Checklist

  • Review Contribution Guidelines
  • Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
  • Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective
  • Provide unit tests (under <subproject>/src/test) to verify logic
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes
  • Ensure that tests pass locally: ./gradlew <changed-subproject>:check

Gradle Core Team Checklist

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation
  • Recognize contributor in release notes

Copy link
Contributor

@oehme oehme left a comment

Choose a reason for hiding this comment

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

This looks good, I just have one stylistic request :)

I'll merge this next week.

* @return true if the path starts with the prefix
*/
public static boolean doesPathStartWith(String path, String startsWithPath) {
// Note: Because this method can be called often during a build, this
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is not necessary, performance tests would catch regressions.

@oehme oehme self-assigned this May 26, 2018
@oehme oehme added in:core a:chore Minor issue without significant impact labels May 26, 2018
@oehme oehme added this to the 4.9 RC1 milestone May 26, 2018
@oehme oehme merged commit 74cebee into gradle:master Jun 1, 2018
@oehme
Copy link
Contributor

oehme commented Jun 1, 2018

Thank you @tjni!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:chore Minor issue without significant impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants