Skip to content

Add concurrency to doclet #139

Closed
lqiu96 wants to merge 15 commits intogoogleapis:mainfrom
lqiu96:concurrent
Closed

Add concurrency to doclet #139
lqiu96 wants to merge 15 commits intogoogleapis:mainfrom
lqiu96:concurrent

Conversation

@lqiu96
Copy link
Copy Markdown
Member

@lqiu96 lqiu96 commented Jul 20, 2022

Fixes #126

It's a good idea to open an issue first for discussion.

  • [ x ] Tests pass
  • [ x ] Appropriate changes to README are included in PR

@lqiu96 lqiu96 requested a review from suztomo July 20, 2022 19:36
Comment on lines +24 to +25
this.globalLookup = new HashMap<>(INITIAL_CAPACITY);
this.localLookupByFileName = new HashMap<>(INITIAL_CAPACITY);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Set a large initial capacity to try and reduce the number of copying done (capacity expansion)

Comment on lines +24 to +25
private final Map<Element, List<? extends Element>> elementMap;
private final Map<Element, List<TypeElement>> elementSortedMap;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Create map to cache the element and the results from getEnclosedElements(). From what I see, most of the slowdown is from element.getEnclosedElement().

Comment on lines -83 to -86
for (int i = 0; i < generatedFileLines.length; i++) {
assertEquals("Wrong file content for file " + generatedFilePath,
generatedFileLines[i], expectedFileLines[i]);
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This portion of the test is removed because with adding concurrency we can't guarantee the ordering of the content in the resulting YML files. Perhaps we can figure out a different way to validate the contents of the YML files.

@lqiu96 lqiu96 removed the request for review from suztomo July 20, 2022 19:47
@lqiu96
Copy link
Copy Markdown
Member Author

lqiu96 commented Jul 20, 2022

Having some concurrency issues. Might need to break out the changes with the Element cache and increase storage for existing caches into a separate PR

@lqiu96 lqiu96 closed this Jul 21, 2022
@lqiu96 lqiu96 mentioned this pull request Jul 25, 2022
2 tasks
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.

mvn javadoc:aggregate -P docFX takes 90 minutes

2 participants