RFR [15] 8239876: Improve SearchIndexItem
Pavel Rappo
pavel.rappo at oracle.com
Tue Feb 25 13:11:53 UTC 2020
Hello,
Please review the change for https://bugs.openjdk.java.net/browse/JDK-8239876:
http://cr.openjdk.java.net/~prappo/8239876/webrev.00/
This is a cleanup change. For convenience I split it into 3 changesets, called
phase-1, phase-2, and phase-3, which are applied in this particular order.
phase-1 changes the way instances of SearchIndexItem are managed.
SearchIndexItem is a POJO whose instances make up the index for the interactive
search feature (JDK-8141492). SearchIndexItem has categories. Before the change
items were stored in the HtmlConfiguration class in 5 (five) different sets, a
set per category. This was both error-prone and not easily extensible. After the
change, there are no sets, but there is a container (SearchIndexItem*s*), which
groups items by category and adapts to new categories automatically.
phase-2 moves a couple of structures, an extra map and a set, used for
alphabetic indexing of instances of SearchIndexItem from HtmlConfiguration to
AbstractIndexWriter. This latter abstract class and its two subclasses,
SplitIndexWriter and SingleIndexWriter, are the only clients of those
structures. This declutters HtmlConfiguration, a weird global object that seems
to have evolved to hold references to everything but the kitchen sink.
phase-3 is a minor cleanup which concludes the change.
***
There are a lot more issues than this change addresses. Here are some of them:
1. SearchIndexItem is mutable and prone to inconsistencies due to not enforcing
any structure and invariants.
2. SearchIndexItem is defined in terms of java.lang.String, where other options
might be more appropriate.
3. SearchIndexItem does not specify the equals and hashCode methods.
Currently, the container relies on comparators when storing items.
Speaking of comparators. There are hidden assumptions about the order of items
in the retrieved collections. A particular order is assumed when SearchIndexItem
instances are merged with elements from IndexBuilder when printing out HTML in
the {Split, Single}IndexWriter classes. That same order is also used when
displaying search results to the user.
4. IndexBuilder and SearchIndexItems are still intertwined. It's not easy to
draw the line where each of them is fully initialized and thus can be used.
Index corresponding to the {@index} and {@systemProperty} tags is put into
SearchIndexItems while traversing classes. Index corresponding to elements is
put into IndexBuilder during its initialization. Then {Split, Single}IndexWriter
updates the SearchIndexItems from IndexBuilder [*] while simultaneously querying
SearchIndexItems to print out the index for the {@index} and {@systemProperty}
tags.
After the change has been applied, the end result will not be ideal.
But it will be definitely better.
-Pavel
-------------------------------------------------------------------------------
[*] This suggests that there's a bug where interactive search results will
contain only {@index} and {@systemProperty} entries if the "-noindex"
command-line option is specified. Surprisingly, the interactive search is
disabled if that option is specified. I guess it's a separate bug. This implicit
dependency between index pages and interactive search has to be straightened
out.
This discovery refers to my recent cleanup
https://mail.openjdk.java.net/pipermail/javadoc-dev/2020-February/001414.html
Namely, "populating that index doesn't seem to have any side-effects". This is
still true. It's just that those side-effects happen to reside in {Split,
Single}IndexWriter, which opportunistically updates the SearchIndexItems
container.
More information about the javadoc-dev
mailing list