hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-5625) Avoid byte buffer allocations when reading a value from a Result object
Date Wed, 25 Apr 2012 16:02:26 GMT

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

jiraposter@reviews.apache.org commented on HBASE-5625:
------------------------------------------------------



bq.  On 2012-04-02 17:34:38, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/KeyValue.java, line 531
bq.  > <https://reviews.apache.org/r/4607/diff/1/?file=97954#file97954line531>
bq.  >
bq.  >     How do we know this buffer is big enough?  Maybe should add an override that
takes an offset into the buffer?
bq.  
bq.  Tudor Scurtu wrote:
bq.      Added overload. Added exception comment for when there is insufficient space remaining
in the buffer.
bq.  
bq.  Michael Stack wrote:
bq.      So, the way this works, we just allocate N and hope that stuff fits inside N?  If
it doesn't we throw an exception?  There is no correlation between data that comes across
and the N allocation?

Please see below.


bq.  On 2012-04-02 17:34:38, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/KeyValue.java, line 616
bq.  > <https://reviews.apache.org/r/4607/diff/1/?file=97954#file97954line616>
bq.  >
bq.  >     How do I know the buffer is big enough?
bq.  
bq.  Tudor Scurtu wrote:
bq.      Added exception comment for when there is insufficient space remaining in the buffer.
Is that what you meant?
bq.  
bq.  Michael Stack wrote:
bq.      I am not understanding how the allocation works.   It seems arbitrary unrelated to
the actual result size that comes over from the server.  Is that so?  If so, it seems unfriendly
throwing an exception when allocated size and what is returned from the server do not match.

Added check with reallocation in 'Result.binarySearch()'. For this I had to add two methods
in 'KeyValue' that calculate the number of bytes that are taken up in a 'KeyValue' object's
underlying buffer ('getKeyValueDataStructureSize()' and 'getKeyDataStructureSize()'). Is this
ok, and if so, how about replacing all manual calculations of these values in the project
with calls to the new methods?


bq.  On 2012-04-02 17:34:38, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/client/Result.java, line 297
bq.  > <https://reviews.apache.org/r/4607/diff/1/?file=97955#file97955line297>
bq.  >
bq.  >     This comment should be on the @return javadoc rather than here.
bq.  
bq.  Tudor Scurtu wrote:
bq.      This was copied from original. Should I remove both occurences?
bq.  
bq.  Michael Stack wrote:
bq.      Sorry.  I did not notice it was problem on original.   If you can fix it, that'd
be sweet.

Fixed.


bq.  On 2012-04-02 17:34:38, Michael Stack wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/client/Result.java, line 431
bq.  > <https://reviews.apache.org/r/4607/diff/1/?file=97955#file97955line431>
bq.  >
bq.  >     Why we have both isNonEmptyColumn and isEmptyColumn?  Why not just one and then
check return with a !?
bq.  
bq.  Tudor Scurtu wrote:
bq.      They are not complementary.
bq.        containsColumn = value exists
bq.        containsEmptyColumn = value exists & is empty byte array
bq.        containsNonEmptyColumn = value exists & is not empty byte array
bq.      The value could be missing, in which case all methods would return false.
bq.  
bq.  Michael Stack wrote:
bq.      OK.  If not clear from comments, please add your notes above.  Will help those that
come after.

Added.


- Tudor


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4607/#review6623
-----------------------------------------------------------


On 2012-04-04 17:08:03, Tudor Scurtu wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/4607/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-04-04 17:08:03)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  When calling Result.getValue(), an extra dummy KeyValue and its associated underlying
byte array are allocated, as well as a persistent buffer that will contain the returned value.
bq.  
bq.  These can be avoided by reusing a static array for the dummy object and by passing a
ByteBuffer object as a value destination buffer to the read method.
bq.  
bq.  
bq.  This addresses bug HBASE-5625.
bq.      https://issues.apache.org/jira/browse/HBASE-5625
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/KeyValue.java 243d76f 
bq.    src/main/java/org/apache/hadoop/hbase/client/Result.java df0b3ef 
bq.    src/test/java/org/apache/hadoop/hbase/TestKeyValue.java fae6902 
bq.    src/test/java/org/apache/hadoop/hbase/client/TestResult.java f9e29c2 
bq.  
bq.  Diff: https://reviews.apache.org/r/4607/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Added value check to TestResult#testBasic and TestResult.testMultiVersion.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Tudor
bq.  
bq.


                
> Avoid byte buffer allocations when reading a value from a Result object
> -----------------------------------------------------------------------
>
>                 Key: HBASE-5625
>                 URL: https://issues.apache.org/jira/browse/HBASE-5625
>             Project: HBase
>          Issue Type: Improvement
>          Components: client
>    Affects Versions: 0.92.1
>            Reporter: Tudor Scurtu
>            Assignee: Tudor Scurtu
>              Labels: patch
>             Fix For: 0.96.0
>
>         Attachments: 5625.txt, 5625v2.txt, 5625v3.txt, 5625v4.txt, 5625v5.txt, 5625v6.txt,
5625v7.txt
>
>
> When calling Result.getValue(), an extra dummy KeyValue and its associated underlying
byte array are allocated, as well as a persistent buffer that will contain the returned value.
> These can be avoided by reusing a static array for the dummy object and by passing a
ByteBuffer object as a value destination buffer to the read method.
> The current functionality is maintained, and we have added a separate method call stack
that employs the described changes. I will provide more details with the patch.
> Running tests with a profiler, the reduction of read time seems to be of up to 40%.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message