[jira] [Commented] (OAK-5092) Add support for weighing the cache entries in caches

["Michael Dürig (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/OAK-5092?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15666800#comment-15666800 ] 

Michael Dürig commented on OAK-5092:
------------------------------------

Thanks for putting this together. I cannot comment on the correctness of the particular size estimations. Maybe you could quickly sketch out what method you used to derive and validate them. 
My main concern is how do we ensure with sufficient certainty that those size estimates remain correct once the underlying entities evolve further? Is there a reasonable way to express this invariant through tests? Somewhat related, I think those magic constants offsetting the weights in the various {{Weigher}} instances and its helper methods should each be accompanied by a comment explaining how that value was derived or what it actually stands for. Otherwise we are going to have a hard time figuring this out later. 

The way {{evictionCount}} was incremented in {{PriorityCache}} was actually correct and is intentional. Have a look at the {{eviction}} flag in the {{put()}} method. Evictions are only replacements of a different entry of the same generation. The other replacements are not interesting and should not be reflected through the cache statistics. 

Instead of {{null}} checking the {{Weigher}} instances, wouldn't it be simpler to make them always {{@Nonnull}} and pass in a trivial implementation by default? Also I'd prefer full names for the {{Weigher}} instances (e.g. {{StringCacheWeigher}} instead of {{StringCacheW}})? 

We should also update {{RecordCacheStats#cacheInfoAsString()}} to include the current weight. 

The new method {{PriorityCache#maxSize()}} is not used anywhere. Is this an oversight or could we remove the method? 

Regarding the casts of the {{Entry}} components in {{PriorityCache}} I would suggest to avoid them by introducing a {{PriorityCache#getEntry(int)}} method and annotate that one to suppress unchecked warnings. 

Finally please also update or add Javadoc where necessary. Also I would like to consistently use the nullability annotations. 

> Add support for weighing the cache entries in caches
> ----------------------------------------------------
>
>                 Key: OAK-5092
>                 URL: https://issues.apache.org/jira/browse/OAK-5092
>             Project: Jackrabbit Oak
>          Issue Type: Improvement
>          Components: segment-tar
>            Reporter: Alex Parvulescu
>            Assignee: Alex Parvulescu
>            Priority: Minor
>             Fix For: 1.6, 1.5.15
>
>         Attachments: OAK-5092.patch
>
>
> Some of the current caches in {{segment-tar}} don't have support for evaluation of current weight. I'd like to look into the {{PriorityCache}} and {{RecordCache}} we use and possibly add something. this is based on initial patch from OAK-4966.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)