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

Improve performance of case-sensitive file key checking #165

Closed
wants to merge 7 commits into from

Conversation

jdimeo
Copy link
Contributor

@jdimeo jdimeo commented Jul 26, 2024

Our Maven site now has about 150,000 Doxia files to render (mostly Markdown and some ASCII doc). During profiling, I realized the Maven site plugin was spending around 18 minutes in the addModuleFiles() method. When looking at the code, I realized the algorithm was O(n² - n / 2) the way it adds each file it discovers to a map and the iterates through the map for each file.

This approach uses a secondary hash map to achieve O(2n) instead to check for a case-insensitive duplicate file to error on Windows and warn on other OSes.

@michael-o
Copy link
Member

@jdimeo Very nice. Can you file a JIRA issue as well, I'd be happy to review this.

@michael-o michael-o self-requested a review August 16, 2024 20:45
@jdimeo
Copy link
Contributor Author

jdimeo commented Aug 17, 2024

@michael-o
Copy link
Member

@jdimeo what us your opinion on the OS check? Should it be extended to macOS as well? HFS+ and APFS are case-insensitive by default.

Copy link
Member

@michael-o michael-o left a comment

Choose a reason for hiding this comment

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

Please rebase on top of master.

@michael-o
Copy link
Member

@jdimeo Please don't forget to rebase. I cannot be merged at this stage.

@jdimeo
Copy link
Contributor Author

jdimeo commented Aug 31, 2024

I'm struggling to do that because apparently a huge number of formatting changes are creating a merge conflict where it's difficult to tell if my commit is being preserved. I tried running the Spotless plugin with the latst pom.xml from master, but it didn't reformat.

@jdimeo
Copy link
Contributor Author

jdimeo commented Aug 31, 2024

I think the problem relates to me using my existing fork from awhile ago and so the rebase is starting with a very very old version.

@jdimeo jdimeo closed this by deleting the head repository Sep 3, 2024
@jdimeo
Copy link
Contributor Author

jdimeo commented Sep 3, 2024

OK I totally started over. Please see: #170
This one reflects the Locale.ROOT change

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.

2 participants