[
https://issues.apache.org/jira/browse/OAK-6915?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16247436#comment-16247436
]
Andrei Dulceanu commented on OAK-6915:
--------------------------------------
[~frm], {{Oak-6915-03.patch}} LGTM. There is one thing though which I think is
done too complicated, i.e. {{SegmentCache#putSegment}}:
{noformat}
SegmentId id = segment.getSegmentId();
Segment internal;
if (id.isDataSegmentId()) {
try {
internal = cache.get(id, newStatsUpdateLoader(id, () -> segment));
} catch (ExecutionException e) {
throw new IllegalStateException("unable to store the segment",
e.getCause());
}
} else {
internal = segment;
}
id.loaded(internal);
{noformat}
Why bother to pass through the cache if we already know the segment is not
there? Another thing here, {{SegmentId#loaded}} is still called after the
segment is put in the cache. I don't know if in the current patch version it
still plays a role, but I was only hinting towards [~mduerig]'s earlier comment.
Personally, I prefer the previous version of {{SegmentCache#putSegment}}:
{noformat}
private void put(@Nonnull SegmentId id, @Nonnull Segment segment) {
// Call loaded *before* putting the segment into the cache as the latter
// might cause it to get evicted right away again.
id.loaded(segment);
cache.put(id, segment);
stats.currentWeight.addAndGet(weigher.weigh(id, segment));
return segment;
}
{noformat}
[~mduerig], [~frm] WDYT?
> Minimize the amount of uncached segment reads
> ---------------------------------------------
>
> Key: OAK-6915
> URL: https://issues.apache.org/jira/browse/OAK-6915
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
> Reporter: Francesco Mari
> Assignee: Francesco Mari
> Fix For: 1.8, 1.7.12
>
> Attachments: OAK-6915-01.patch, OAK-6915-02.patch, OAK-6915-03.patch,
> OAK-6915-diagnostics-02.patch, OAK-6915-diagnostics.patch, OAK-6915.patch,
> Screen Shot 2017-11-09 at 14.14.28.png, Screen Shot 2017-11-09 at 14.16.59.png
>
>
> The current implementation of {{SegmentCache}} should make better use of the
> underlying Guava cache by relying on the cached segments instead of
> unconditionally performing an uncached segment read via the
> {{Callable<Segment>}} passed to {{SegmentCache#getSegment}}.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
