Juan Hernandez has posted comments on this change. Change subject: restapi: optimize getUriBuilder ......................................................................
Patch Set 1: (2 comments) http://gerrit.ovirt.org/#/c/37988/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 800: /** Line 801: * Cache the {@code UriBuilder}s by the associated model so we don't have to create the builders each time. Line 802: */ Line 803: private static Map<Class<? extends BaseResource>, UriBuilder> builderMap = Line 804: new ConcurrentHashMap<Class<? extends BaseResource>, UriBuilder>(); > We might even not need the ConcurrentHashMap here. It is needed, the behaviour of a plain HashMap is undefined when concurrently reading and writing. Line 805: Line 806: /** Line 807: * Create a #UriBuilder which encapsulates the path to an object Line 808: * Line 843: } else { Line 844: //We need to clone so we have our own copy to work with. Cloning is much faster than building a new one Line 845: //from scratch each time. Line 846: uriBuilder = uriBuilder.clone(); Line 847: } I'm not sure if this will work correctly in all cases. Take into account that state of the UriBuilder doesn't depend only on the class of the model, but also on the classes of the parent models. A model, for example a Disk, may be used in multiple places: in the /disks collection (no parent), in the /vm/{vm:id}/disks collection (the parent is the VM), in the /storagedomain/{storagedomain:id}/disks collection (the parent is the Storage Domain), etc. In these cases if we cache the builder based only in the model class we will be calculating the URL incorrectly in all but one of the scenarios. Line 848: Line 849: return uriBuilder.path(model.getId()); Line 850: } Line 851: -- To view, visit http://gerrit.ovirt.org/37988 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I86221e8af24da28a7137c0cc0e7aec69943a65c5 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Einav Cohen <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
