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

Use static concurrent cache for dedupe. #1016

Closed

Conversation

anuraaga
Copy link
Contributor

@anuraaga anuraaga commented Dec 6, 2021

  • a summary of the change
  • either
    • a link to the issue you are resolving (for small changes)
    • a link to the PR you just created (for big changes likely to have discussion)

I first tried changing the Map to concurrent but it didn't fix the issue, making it static did though. If making it static has potential issues, an alternative might be something like attaching the provisioner to Project in newDedupingProvisioner to reference to dedupe the deduper itself, which was probably the root problem. I have left the map as concurrent though since I don't think Gradle's threading model is clear enough to safely use a non-concurrent one here.

I have run my build with a local snapshot and could confirm it passes with this change.

Fixes #1015

@anuraaga anuraaga changed the title Use static concurrent cache for dedupe prevention. Use static concurrent cache for dedupe. Dec 6, 2021
@nedtwigg
Copy link
Member

nedtwigg commented Dec 6, 2021

This is a good fix, but I think it masks the root cause. Superceded by #1020. If you want the performance increase that this centralized approach provides, happy to take a PR for #984, but I'm skeptical that it would actually be faster.

@nedtwigg nedtwigg closed this Dec 6, 2021
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.

Spotless Gradle 6.0.0 -> 6.0.2 ConcurrentModificationException bug for large parallel builds
2 participants