Yair Zaslavsky has posted comments on this change.
Change subject: engine: Add caching capabilities to DAOs
......................................................................
Patch Set 1:
(3 comments)
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/EntityCache.java
Line 9:
Line 10: public class EntityCache {
Line 11:
Line 12: private static final EntityCache instance = new EntityCache();
Line 13: private Map<EntityCacheKey, EntityCacheItem> cache = new
HashMap<EntityCacheKey, EntityCacheItem>();
Maybe you should change the interface to be ConcurrentMap, and the implementing
class to be ConcurrentHashMap? See my next comment.
Line 14:
Line 15: private EntityCache() {
Line 16:
Line 17: }
Line 21: }
Line 22:
Line 23: public <T extends CachedEntity> T getFromCache(Guid id, ReadDao
dao) {
Line 24: EntityCacheKey key = new EntityCacheKey(id, dao);
Line 25: T instance = EntityCache.getInstance().get(key);
Maybe you should replace this code with putIfAbsent usage of ConcurentMap?
Line 26: if (instance == null) {
Line 27: instance = (T) dao.get(id);
Line 28: EntityCache.getInstance().put(key, instance);
Line 29: }
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml
Line 47: <include
name="common/businessentities/DiskAlignment.java" />
Line 48: <include
name="common/businessentities/OpenStackImageProviderProperties.java" />
Line 49: <include name="common/businessentities/VmBalloonInfo.java" />
Line 50: <include
name="common/businessentities/ArchitectureType.java" />
Line 51: <include
name="common/businessentities/CachedEntity.java" />
hmm.... :/ I don't like the fact we need to exposed CachedEntity - I
understand why technically we need to do it.
An alternative approach might be to hold "expiration table" - have at the
EntityCache a table that maps a class to the expiration time of its objects,
and usage that for expliration calculation, and then you will be able to
eliminate CachedEntity, and you won't need to expose any Caching information to
GWT.
Your thoughts here?
Line 52:
Line 53: <!-- Network business entities -->
Line 54: <include
name="common/businessentities/network/VdsNetworkInterface.java" />
Line 55: <include
name="common/businessentities/network/NetworkInterface.java" />
--
To view, visit http://gerrit.ovirt.org/21952
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3354858d67f20661363cccc60380119c30e12ba
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[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
