Sarah Jelinek has posted comments on this change.

Change subject: WIP: refactor cache a bit for contiguous key/value
......................................................................


Patch Set 1:

(1 comment)

Just a few new comments.

http://gerrit.cloudera.org:8080/#/c/2326/1/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:

Line 345:     scratch.TryAllocateFromCache(cache, key, ptr.size());
> You are only allocating ptr.size() here. And I don't want the whole key str
Actually, I see that you are allocating sufficient memory but it looks like you 
are setting the scratch pointer to the first address in scratch, rather than 
scratch + key. NVM stores the key in the first part of that memory location, 
then the value. Where are you copying the key value into the scratch pointer 
address?

XXX update, nevermind, I am wrong. I see you copying it in the 
BlockCache::Allocate call.


-- 
To view, visit http://gerrit.cloudera.org:8080/2326
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic822d0020431f8d3233034af0cd11c4f2f448a1e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sarah Jelinek <[email protected]>
Gerrit-HasComments: Yes

Reply via email to