Hi Hannes,

I've updated the webrev:

  http://cr.openjdk.java.net/~prappo/8239243/webrev.01/

Thanks!

> On 18 Feb 2020, at 09:52, Hannes Wallnöfer <[email protected]> 
> wrote:
> 
> 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