RFR [15] 8239243: Create index structures only if required

[Pavel Rappo]

Thanks for looking into this.

> On 18 Feb 2020, at 22:57, Jonathan Gibbons <jonathan.gibbons at oracle.com> wrote:
> 
> In a number of places, you use a variable or field named "indexbuilder" that would be better written in all situations as "indexBuilder".  No need for re-review to fix that.

That practice predates this fix, but I'll update the fields as requested.

> The work is good, for what it does, but it triggers a discussion for a potential followup.   What follows is a discussion for future work, not necessarily part of this review.

That exact follow-up you are talking about has been in the works for some time now.
Stay tuned.

> "IndexBuilder" is "old" and now is only half the story. It's the A-Z index, and is unrelated to the interactive search index.  There's a bunch of fields in HtmlConfiguration that could arguably be moved/merged into an imporved abstraction of IndexBuilder that is able to manage the A-Z index as well as the interactive index. Either that, or, IndexBuilder is renamed to AZIndexBuilder, and we create a new sibling structure for the interactive search.    For the fields in question, look at this list from HtmlConfiguration:
> 
> protected SortedSet<SearchIndexItem> memberSearchIndex
> ;
> 
> protected SortedSet<SearchIndexItem> moduleSearchIndex
> ;
> 
> protected SortedSet<SearchIndexItem> packageSearchIndex
> ;
> 
> protected SortedSet<SearchIndexItem> tagSearchIndex
> ;
> 
> protected SortedSet<SearchIndexItem> typeSearchIndex
> ;
> 
> protected Map<Character,List<SearchIndexItem>> tagSearchIndexMap = new 
> HashMap<>();
> 
> protected Set<Character> tagSearchIndexKeys;
> These fields, and probably much of the code to manage them should be moved out of HtmlConfiguration into either IndexBuilder or a some new abstraction to manage these index objects.  This would be philosophically similar to the recent cleanup to move the collections of fields for options out into distinct abstractions. 
> 
> -- Jon
> 
> On 02/18/2020 07:13 AM, Pavel Rappo wrote:
>> 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 <hannes.wallnoefer at oracle.com>
>>>  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 <pavel.rappo at oracle.com>
>>>> :
>>>> 
>>>> 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
>>>> 
>>>> 
>