Yingyi Bu has posted comments on this change.

Change subject: BufferCache Concurrency Fixes
......................................................................


Patch Set 2:

(2 comments)

https://asterix-gerrit.ics.uci.edu/#/c/780/2/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
File 
hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java:

Line 994:                     victim.dpid = -1;
It seems that setting dpid to -1 will bypass the code from line 321 to line 419.

I'm not sure if the new code here (line 966 to line 995) has the same behavior 
as the the old code, for the case that the new file id is different from the 
old file id.  How does the new code deal with pin count?


https://asterix-gerrit.ics.uci.edu/#/c/780/2/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java
File 
hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java:

Line 134:         return clockPtr.getAndUpdate(value -> (value + 1) % 
numPages.get());
I'm not very sure whether the Lambda expression will create a new object every 
time here.  The Java specification seems vague on that:
http://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.27.4

This is a frequently executed code path, therefore we need to make sure there 
is no new object creation here.


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/780
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ife86104c5d1dde597e1a990b7207fda4d4bef039
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Michael Blow <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-HasComments: Yes

Reply via email to