Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12825 )
Change subject: WIP [util] introduce TTL cache ...................................................................... Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/12825/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12825/2//COMMIT_MSG@18 PS2, Line 18: * maybe, update the behavior of the cache : to purge expired items upon lookup +1, but would be curious if you've got an argument for not doing this. http://gerrit.cloudera.org:8080/#/c/12825/2/src/kudu/util/ttl_cache.h File src/kudu/util/ttl_cache.h: http://gerrit.cloudera.org:8080/#/c/12825/2/src/kudu/util/ttl_cache.h@41 PS2, Line 41: entiries nit: entries http://gerrit.cloudera.org:8080/#/c/12825/2/src/kudu/util/ttl_cache.h@42 PS2, Line 42: do nit: does http://gerrit.cloudera.org:8080/#/c/12825/2/src/kudu/util/ttl_cache.h@63 PS2, Line 63: V& value() const { : DCHECK(ptr_); : return *ptr_; : } Is this used anywhere right now? http://gerrit.cloudera.org:8080/#/c/12825/2/src/kudu/util/ttl_cache.h@109 PS2, Line 109: // If the item has expired already, return null handle. The underlying : // cache will purge the expired entry when necessary upon accommodating : // new entries being added into the cache, if any. Curious what the line of thought behind this is -- it seems innocuous enough to purge now? I guess it saves on some removal calls up to the point where we're full, but doing so consumes more memory for those expired entries (albeit bounded by our configured limit). http://gerrit.cloudera.org:8080/#/c/12825/2/src/kudu/util/ttl_cache.h@128 PS2, Line 128: size_t charge Perhaps it's worth having a default of kAutomaticCharge? -- To view, visit http://gerrit.cloudera.org:8080/12825 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic8aa0ebe6b26bf34ca0e02bebbdb6a94f6b00621 Gerrit-Change-Number: 12825 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 23 Mar 2019 00:06:34 +0000 Gerrit-HasComments: Yes
