Hi Pavel,

+1 for the conditional index building, and the IndexBuilder cleanup is a major 
improvement in every respect.

Nitpicking here, but two naming suggestions:

- „index“ can be both verb and noun, so renaming the „index“ method to 
„indexElements“ might make its meaning a bit clearer and puts it in line with 
the other index* methods

- since firstCharacter(String) doesn’t always return the first character maybe 
something like „indexCharacter“ or „keyCharacter“ would be more fitting?

Hannes


> Am 17.02.2020 um 16:35 schrieb Pavel Rappo <[email protected]>:
> 
> Hello,
> 
> Please review the change for https://bugs.openjdk.java.net/browse/JDK-8239243:
> 
>  http://cr.openjdk.java.net/~prappo/8239243/webrev.00/
> 
> I noticed that the index structure, used in Index Page an others, is created
> unconditionally (i.e. regardless of the presence of the "-noindex" 
> command-line
> option). Since populating that index doesn't seem to have any side-effects, 
> this
> will unnecessarily consume resources of documentation generation process when 
> no
> index is required. Thus, moving it under the relevant if-statement should be
> safe and the right thing to do. I've also cleaned up the IndexBuilder class a 
> bit.
> 
> In case you wonder, when generating the API docs for the JDK 15, it takes 
> about
> 1 sec. (one second) to create that structure. The number of instances of 
> Element
> in that Map<Character, SortedSet<Element>> is roughly 50,000 (fifty thousand).
> 
> -Pavel
> 

Reply via email to