Juan Hernandez has posted comments on this change.
Change subject: core: Cache GUID creation
......................................................................
Patch Set 1: (1 inline comment)
....................................................
File
backend/manager/modules/compat/src/main/java/org/ovirt/engine/core/compat/NGuid.java
Line 20: 5, 4, 7, 6, 8, 9, 10, 11, 12, 13, 14, 15 };
Line 21: private static final byte[] KEEP_BYTE_ORDER_INDICES = { 0, 1, 2, 3,
Line 22: 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 };
Line 23:
Line 24: private Map<String, UUID> cachedGuids = new HashMap<String,
UUID>();
Shouldn't this be static? Otherwise we end up with one cache per instance of
NGuid, so no cache at all. Am I missing something?
And if it is static then it needs to be a ConcurrentHashMap or else the
createGuidOrGetFromCache methods needs to be synchronized, as the HashMap is
not multi-thread safe.
Also the cache needs to be cleaned up, otherwise this is a leak waiting to byte
us. For the server side I would consider using Infispan, but that is not
possible in the GUI.
Also I guess that the really expensive operation here is the conversion from
UUID to string and the other way around, this is what we should strive to avoid.
All in all, this cache may end up being more expensive than generating a new
UUID each time. I would suggest to refrain from using this kind of cache unless
numbers clearly show a performance improvement.
Line 25:
Line 26: public final static Guid Empty = new Guid();
Line 27:
Line 28: public static Guid NewGuid() {
--
To view, visit http://gerrit.ovirt.org/16079
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I49a103f6194d9c4b703c1bc965b5a56bba4e9634
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
