Hi,
I thought about improving performance of the default doclet implementation a
bit.
According to profiler results, most of the CPU time is spent in constructing
the member map in VisibleMemberMap.java.
So I've rewritten part of the VisbleMemberMap and reduced the javadoc run time
to 60-70% of original run time (on large inputs).
Is there any chance of this patch (attached) being accepted into OpenJDK?
I've never made any contribution to OpenJDK before, so I would like to ask for
code review and guidance through the contribution process.
Michael
diff --git a/src/share/classes/com/sun/tools/doclets/internal/toolkit/util/VisibleMemberMap.java b/src/share/classes/com/sun/tools/doclets/internal/toolkit/util/VisibleMemberMap.java
--- a/src/share/classes/com/sun/tools/doclets/internal/toolkit/util/VisibleMemberMap.java
+++ b/src/share/classes/com/sun/tools/doclets/internal/toolkit/util/VisibleMemberMap.java
@@ -48,8 +48,6 @@
*/
public class VisibleMemberMap {
- private boolean noVisibleMembers = true;
-
public static final int INNERCLASSES = 0;
public static final int ENUM_CONSTANTS = 1;
public static final int FIELDS = 2;
@@ -70,23 +68,15 @@
* List of ClassDoc objects for which ClassMembers objects are built.
*/
private final List<ClassDoc> visibleClasses = new ArrayList<ClassDoc>();
-
+
/**
- * Map for each member name on to a map which contains members with same
- * name-signature. The mapped map will contain mapping for each MemberDoc
- * onto it's respecive level string.
+ * Maps classes to their visible members
*/
- private final Map<Object,Map<ProgramElementDoc,String>> memberNameMap = new HashMap<Object,Map<ProgramElementDoc,String>>();
-
+ private final Map<ClassDoc, List<ProgramElementDoc>> memberMap = new HashMap<>();
+
/**
* Map of class and it's ClassMembers object.
*/
- private final Map<ClassDoc,ClassMembers> classMap = new HashMap<ClassDoc,ClassMembers>();
-
- /**
- * Type whose visible members are requested. This is the leaf of
- * the class tree being mapped.
- */
private final ClassDoc classdoc;
/**
@@ -123,7 +113,7 @@
this.classdoc = classdoc;
this.kind = kind;
this.configuration = configuration;
- new ClassMembers(classdoc, STARTLEVEL).build();
+ new MapBuilder().build();
}
/**
@@ -206,11 +196,7 @@
* @return the list of members for the given class.
*/
public List<ProgramElementDoc> getMembersFor(ClassDoc cd) {
- ClassMembers clmembers = classMap.get(cd);
- if (clmembers == null) {
- return new ArrayList<ProgramElementDoc>();
- }
- return clmembers.getMembers();
+ return memberMap.get(cd);
}
/**
@@ -233,159 +219,63 @@
list.addAll(interfaces);
}
- private void fillMemberLevelMap(List<ProgramElementDoc> list, String level) {
- for (int i = 0; i < list.size(); i++) {
- Object key = getMemberKey(list.get(i));
- Map<ProgramElementDoc,String> memberLevelMap = memberNameMap.get(key);
- if (memberLevelMap == null) {
- memberLevelMap = new HashMap<ProgramElementDoc,String>();
- memberNameMap.put(key, memberLevelMap);
- }
- memberLevelMap.put(list.get(i), level);
- }
- }
+ private class MapBuilder {
+ Map<ClassDoc, Integer> levelMap = new HashMap<>();
- private void purgeMemberLevelMap(List<ProgramElementDoc> list, String level) {
- for (int i = 0; i < list.size(); i++) {
- Object key = getMemberKey(list.get(i));
- Map<ProgramElementDoc, String> memberLevelMap = memberNameMap.get(key);
- if (level.equals(memberLevelMap.get(list.get(i))))
- memberLevelMap.remove(list.get(i));
- }
- }
-
- /**
- * Represents a class member. We should be able to just use a
- * ProgramElementDoc instead of this class, but that doesn't take
- * type variables in consideration when comparing.
- */
- private class ClassMember {
- private Set<ProgramElementDoc> members;
-
- public ClassMember(ProgramElementDoc programElementDoc) {
- members = new HashSet<ProgramElementDoc>();
- members.add(programElementDoc);
+ public void build() {
+ fillMap(classdoc, new HashMap<String, ProgramElementDoc>(), 0);
+ visibleClasses.addAll(levelMap.keySet());
}
- public void addMember(ProgramElementDoc programElementDoc) {
- members.add(programElementDoc);
- }
-
- public boolean isEqual(MethodDoc member) {
- for (Iterator<ProgramElementDoc> iter = members.iterator(); iter.hasNext(); ) {
- MethodDoc member2 = (MethodDoc) iter.next();
- if (Util.executableMembersEqual(member, member2)) {
- members.add(member);
- return true;
+ private void fillMap(ClassDoc clazz, Map<String, ProgramElementDoc> memberAcc, int level) {
+ int thatLevel = 0;
+ if (!levelMap.containsKey(clazz)) {
+ levelMap.put(clazz, level);
+ } else {
+ thatLevel = levelMap.get(clazz);
+ if (thatLevel < level) {
+ levelMap.put(clazz, level);
}
}
- return false;
- }
- }
- /**
- * A data structure that represents the class members for
- * a visible class.
- */
- private class ClassMembers {
+ if (thatLevel <= level) {
+ List<ProgramElementDoc> members = new ArrayList<>();
+ for (ProgramElementDoc member : getClassMembers(clazz, true)) {
+ if (memberIsVisible(member) && !isTreatedAsPrivate(member)) {
+ String key = member.name();
+ if (member instanceof ConstructorDoc) {
+ members.add(member);
+ } else if (member instanceof MethodDoc) {
+ if (!memberAcc.containsKey(key)
+ || !Util.executableMembersEqual((MethodDoc) member, (MethodDoc) memberAcc.get(key))) {
+ memberAcc.put(key, member);
+ members.add(member);
+ }
+ } else {
+ if (!memberAcc.containsKey(key)) {
+ memberAcc.put(key, member);
+ members.add(member);
+ }
+ }
+ }
+ }
- /**
- * The mapping class, whose inherited members are put in the
- * {@link #members} list.
- */
- private ClassDoc mappingClass;
-
- /**
- * List of inherited members from the mapping class.
- */
- private List<ProgramElementDoc> members = new ArrayList<ProgramElementDoc>();
-
- /**
- * Level/Depth of inheritance.
- */
- private String level;
-
- /**
- * Return list of inherited members from mapping class.
- *
- * @return List Inherited members.
- */
- public List<ProgramElementDoc> getMembers() {
- return members;
- }
-
- private ClassMembers(ClassDoc mappingClass, String level) {
- this.mappingClass = mappingClass;
- this.level = level;
- if (classMap.containsKey(mappingClass) &&
- level.startsWith(classMap.get(mappingClass).level)) {
- //Remove lower level class so that it can be replaced with
- //same class found at higher level.
- purgeMemberLevelMap(getClassMembers(mappingClass, false),
- classMap.get(mappingClass).level);
- classMap.remove(mappingClass);
- visibleClasses.remove(mappingClass);
- }
- if (!classMap.containsKey(mappingClass)) {
- classMap.put(mappingClass, this);
- visibleClasses.add(mappingClass);
+ if (level != 0 && thatLevel == level) {
+ memberMap.get(clazz).retainAll(members);
+ } else {
+ memberMap.put(clazz, members);
+ }
}
- }
+ if (clazz.isClass() && clazz.superclass() != null) {
+ fillMap(clazz.superclass(), memberAcc, level + 1);
+ }
- private void build() {
- if (kind == CONSTRUCTORS) {
- addMembers(mappingClass);
- } else {
- mapClass();
+ for (ClassDoc inf : clazz.interfaces()) {
+ fillMap(inf, new HashMap<>(memberAcc), level + 1);
}
}
- private void mapClass() {
- addMembers(mappingClass);
- ClassDoc[] interfaces = mappingClass.interfaces();
- for (int i = 0; i < interfaces.length; i++) {
- String locallevel = level + 1;
- ClassMembers cm = new ClassMembers(interfaces[i], locallevel);
- cm.mapClass();
- }
- if (mappingClass.isClass()) {
- ClassDoc superclass = mappingClass.superclass();
- if (!(superclass == null || mappingClass.equals(superclass))) {
- ClassMembers cm = new ClassMembers(superclass,
- level + "c");
- cm.mapClass();
- }
- }
- }
-
- /**
- * Get all the valid members from the mapping class. Get the list of
- * members for the class to be included into(ctii), also get the level
- * string for ctii. If mapping class member is not already in the
- * inherited member list and if it is visible in the ctii and not
- * overridden, put such a member in the inherited member list.
- * Adjust member-level-map, class-map.
- */
- private void addMembers(ClassDoc fromClass) {
- List<ProgramElementDoc> cdmembers = getClassMembers(fromClass, true);
- List<ProgramElementDoc> incllist = new ArrayList<ProgramElementDoc>();
- for (int i = 0; i < cdmembers.size(); i++) {
- ProgramElementDoc pgmelem = cdmembers.get(i);
- if (!found(members, pgmelem) &&
- memberIsVisible(pgmelem) &&
- !isOverridden(pgmelem, level) &&
- !isTreatedAsPrivate(pgmelem)) {
- incllist.add(pgmelem);
- }
- }
- if (incllist.size() > 0) {
- noVisibleMembers = false;
- }
- members.addAll(incllist);
- fillMemberLevelMap(getClassMembers(fromClass, false), level);
- }
-
private boolean isTreatedAsPrivate(ProgramElementDoc pgmelem) {
if (!configuration.javafx) {
return false;
@@ -496,39 +386,6 @@
return targetMembers.toArray(new AnnotationTypeElementDoc[]{});
}
- private boolean found(List<ProgramElementDoc> list, ProgramElementDoc elem) {
- for (int i = 0; i < list.size(); i++) {
- ProgramElementDoc pgmelem = list.get(i);
- if (Util.matches(pgmelem, elem)) {
- return true;
- }
- }
- return false;
- }
-
-
- /**
- * Is member overridden? The member is overridden if it is found in the
- * same level hierarchy e.g. member at level "11" overrides member at
- * level "111".
- */
- private boolean isOverridden(ProgramElementDoc pgmdoc, String level) {
- Map<?,String> memberLevelMap = (Map<?,String>) memberNameMap.get(getMemberKey(pgmdoc));
- if (memberLevelMap == null)
- return false;
- String mappedlevel = null;
- Iterator<String> iterator = memberLevelMap.values().iterator();
- while (iterator.hasNext()) {
- mappedlevel = iterator.next();
- if (mappedlevel.equals(STARTLEVEL) ||
- (level.startsWith(mappedlevel) &&
- !level.equals(mappedlevel))) {
- return true;
- }
- }
- return false;
- }
-
private ProgramElementDoc[] properties(final ClassDoc cd, final boolean filter) {
final MethodDoc[] allMethods = cd.methods(filter);
final FieldDoc[] allFields = cd.fields(false);
@@ -656,7 +513,7 @@
}
return null;
}
-
+
// properties aren't named setA* or getA*
private final Pattern pattern = Pattern.compile("[sg]et\\p{Upper}.*");
private boolean isPropertyMethod(MethodDoc method) {
@@ -736,36 +593,6 @@
* @return true if this map has no visible members.
*/
public boolean noVisibleMembers() {
- return noVisibleMembers;
- }
-
- private ClassMember getClassMember(MethodDoc member) {
- for (Iterator<?> iter = memberNameMap.keySet().iterator(); iter.hasNext();) {
- Object key = iter.next();
- if (key instanceof String) {
- continue;
- } else if (((ClassMember) key).isEqual(member)) {
- return (ClassMember) key;
- }
- }
- return new ClassMember(member);
- }
-
- /**
- * Return the key to the member map for the given member.
- */
- private Object getMemberKey(ProgramElementDoc doc) {
- if (doc.isConstructor()) {
- return doc.name() + ((ExecutableMemberDoc)doc).signature();
- } else if (doc.isMethod()) {
- return getClassMember((MethodDoc) doc);
- } else if (doc.isField() || doc.isEnumConstant() || doc.isAnnotationTypeElement()) {
- return doc.name();
- } else { // it's a class or interface
- String classOrIntName = doc.name();
- //Strip off the containing class name because we only want the member name.
- classOrIntName = classOrIntName.indexOf('.') != 0 ? classOrIntName.substring(classOrIntName.lastIndexOf('.'), classOrIntName.length()) : classOrIntName;
- return "clint" + classOrIntName;
- }
+ return memberMap.isEmpty();
}
}
