Juan Hernandez has posted comments on this change. Change subject: core: Long response time when using list API ......................................................................
Patch Set 1: (6 comments) https://gerrit.ovirt.org/#/c/40809/1//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2015-05-12 13:23:02 +0300 Line 4: Commit: lzel...@redhat.com <lzel...@redhat.com> Line 5: CommitDate: 2015-05-12 13:29:07 +0300 Line 6: Line 7: core: Long response time when using list API Replace "core" with "restapi". Line 8: Line 9: Cache some reflection work so that list API serialization will be faster. Line 10: According to the profiler, this should save ~33% of serialization time. Line 11: https://gerrit.ovirt.org/#/c/40809/1/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/LinkHelper.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/LinkHelper.java: Line 311: /** Line 312: * A map describing every possible collection Line 313: */ Line 314: private static ModelToCollectionsMap TYPES = new ModelToCollectionsMap(); Line 315: private static ConcurrentMap<Class<?>, List<Method>> methodCache = new ConcurrentHashMap<>(); Consider adding a blank line and a javadoc comment explaining what this map is used for. Line 316: Line 317: static { Line 318: ParentToCollectionMap map; Line 319: Line 705: } Line 706: Line 707: /** Line 708: * Gets all the relevant possible inline resources methods of a class. Data is cached for future use. Line 709: * @param clz This @param is useless if empty. Line 710: * @return The list of relevant methods. Line 711: */ Line 712: private static List<Method> getRelevantMethods(Class<?> clz) { Line 713: List<Method> methods = new ArrayList<>(); Line 709: * @param clz Line 710: * @return The list of relevant methods. Line 711: */ Line 712: private static List<Method> getRelevantMethods(Class<?> clz) { Line 713: List<Method> methods = new ArrayList<>(); This variable shouldn't be created here, as in most cases it won't be used. Line 714: if (methodCache.containsKey(clz)) { Line 715: return methodCache.get(clz); Line 716: } Line 717: for (Method method : clz.getMethods()) { Line 711: */ Line 712: private static List<Method> getRelevantMethods(Class<?> clz) { Line 713: List<Method> methods = new ArrayList<>(); Line 714: if (methodCache.containsKey(clz)) { Line 715: return methodCache.get(clz); This performs two lookups in the map, which is expensive. Try to perform only one: List<Method> methods = methodCache.get(clz); if (methods != null) { return methods; } methods = new ArrayList<>(); ... Line 716: } Line 717: for (Method method : clz.getMethods()) { Line 718: if (method.getName().startsWith("get")) { Line 719: if (method.getReturnType().getPackage() == BaseResource.class.getPackage()) { Line 712: private static List<Method> getRelevantMethods(Class<?> clz) { Line 713: List<Method> methods = new ArrayList<>(); Line 714: if (methodCache.containsKey(clz)) { Line 715: return methodCache.get(clz); Line 716: } The "methods" list should be created here. Line 717: for (Method method : clz.getMethods()) { Line 718: if (method.getName().startsWith("get")) { Line 719: if (method.getReturnType().getPackage() == BaseResource.class.getPackage()) { Line 720: methods.add(method); -- To view, visit https://gerrit.ovirt.org/40809 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1801e5589d8cec1474e32e495b35b6c150e101ca Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
