[
https://issues.apache.org/jira/browse/HBASE-4027?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13084264#comment-13084264
]
[email protected] commented on HBASE-4027:
------------------------------------------------------
bq. On 2011-08-09 22:05:13, Todd Lipcon wrote:
bq. > src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java, line
114
bq. > <https://reviews.apache.org/r/1214/diff/8/?file=31773#file31773line114>
bq. >
bq. > > 0, not == 1 -- the contract of compareTo is just that it returns
positive, not that it returns exactly 1
bq.
bq. Li Pi wrote:
bq.
http://download.oracle.com/javase/6/docs/api/java/math/BigDecimal.html#compareTo(java.math.BigDecimal)
bq.
bq. Returns:
bq. -1, 0, or 1 as this BigDecimal is numerically less than, equal to, or
greater than val.
bq.
bq.
bq. Todd Lipcon wrote:
bq.
http://download.oracle.com/javase/1.4.2/docs/api/java/lang/Comparable.html#compareTo(java.lang.Object)
bq.
bq. "Returns:
bq. a negative integer, zero, or a positive integer as this object is less
than, equal to, or greater than the specified object."
bq.
bq. so even though sun's BigDecimal happens to return exactly 1, the
convention is to check compareTo(...) > 0, because the usual interface is
pos/zero/negative, not 1/0/-1
Okay. Will fix. Thanks!
bq. On 2011-08-09 22:05:13, Todd Lipcon wrote:
bq. >
src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java, lines
101-103
bq. > <https://reviews.apache.org/r/1214/diff/8/?file=31771#file31771line101>
bq. >
bq. > when you check up front here, you end up doing two lookups in
backingmap. Since this is just a safety check, you could instead check the
return value of put() below. Something like:
bq. >
bq. > ByteBuffer storedBlock = ...allloc
bq. > ... fill it in...
bq. > ByteBuffer alreadyCached = backingMap.put(blockName, storedBlock);
bq. > if (alreadyCached != null) {
bq. > // we didn't insert the new one, so free it and throw an exception
bq. > backingStore.free(storedBlock);
bq. > throw new RuntimeException("already cached xxxxx");
bq. > }
bq. >
bq. > make sense?
bq.
bq. Li Pi wrote:
bq. Doesn't put overwrite the previous value? I guess in this case, it
doesn't matter, because you'd just be caching the same thing twice. According
to mapmaker: "If the map previously contained a mapping for the key, the old
value is replaced by the specified value."
bq.
bq. Good change, still though, I changed it to free alreadyCached instead.
bq.
bq. Todd Lipcon wrote:
bq. ah, sorry, I meant putIfAbsent from ConcurrentMap. If you free
alreadyCached, you might free something that someone's using, right?
Yes, but that someone else stores a reference to the singlesizecache and a copy
of the key. They'll look it up by key again, and will hit the new item.
bq. On 2011-08-09 22:05:13, Todd Lipcon wrote:
bq. > src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java,
line 64
bq. > <https://reviews.apache.org/r/1214/diff/8/?file=31767#file31767line64>
bq. >
bq. > does it ever make sense to have offHeapSize < onHeapSize? Perhaps we
should have a Preconditions check here?
bq.
bq. Li Pi wrote:
bq. Its useful for testing, I guess? And 8gb heap with 2gb of offheap >
8gb heap without 2gb of offheap.
bq.
bq. Todd Lipcon wrote:
bq. is that true? wouldn't the 2G offheap always have a subset of what's
in heap?
...ah yes, good point. i'll add this check in.
- Li
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1214/#review1363
-----------------------------------------------------------
On 2011-08-12 08:41:37, Li Pi wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/1214/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-08-12 08:41:37)
bq.
bq.
bq. Review request for hbase, Todd Lipcon, Ted Yu, Michael Stack, Jonathan
Gray, and Li Pi.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Review request - I apparently can't edit tlipcon's earlier posting of my
diff, so creating a new one.
bq.
bq.
bq. This addresses bug HBase-4027.
bq. https://issues.apache.org/jira/browse/HBase-4027
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. conf/hbase-env.sh 2d55d27
bq. src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 2d4002c
bq. src/main/java/org/apache/hadoop/hbase/io/hfile/CacheStats.java
PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java
PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java 097dc50
bq. src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
1338453
bq. src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java
886c31d
bq. src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java
PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/io/hfile/slab/Slab.java
PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java
PRE-CREATION
bq.
src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabItemEvictionWatcher.java
PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
7a917da
bq. src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
7b7bf73
bq. src/main/java/org/apache/hadoop/hbase/util/DirectMemoryUtils.java
PRE-CREATION
bq.
src/test/java/org/apache/hadoop/hbase/io/hfile/HFileBlockCacheTestUtils.java
PRE-CREATION
bq.
src/test/java/org/apache/hadoop/hbase/io/hfile/SingleSizeCacheTestUtils.java
PRE-CREATION
bq.
src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSingleSizeCache.java
PRE-CREATION
bq. src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSlab.java
PRE-CREATION
bq. src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSlabCache.java
PRE-CREATION
bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
4387170
bq.
bq. Diff: https://reviews.apache.org/r/1214/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Ran benchmarks against it in HBase standalone mode. Wrote test cases for
all classes, multithreaded test cases exist for the cache.
bq.
bq.
bq. Thanks,
bq.
bq. Li
bq.
bq.
> Enable direct byte buffers LruBlockCache
> ----------------------------------------
>
> Key: HBASE-4027
> URL: https://issues.apache.org/jira/browse/HBASE-4027
> Project: HBase
> Issue Type: Improvement
> Reporter: Jason Rutherglen
> Assignee: Li Pi
> Priority: Minor
> Attachments: 4027-v5.diff, 4027v7.diff, HBase-4027 (1).pdf,
> HBase-4027.pdf, HBase4027v8.diff, HBase4027v9.diff, hbase-4027-v10.5.diff,
> hbase-4027-v10.diff, hbase-4027v10.6.diff, hbase-4027v6.diff,
> hbase4027v11.5.diff, hbase4027v11.diff, slabcachepatch.diff,
> slabcachepatchv2.diff, slabcachepatchv3.1.diff, slabcachepatchv3.2.diff,
> slabcachepatchv3.diff, slabcachepatchv4.5.diff, slabcachepatchv4.diff
>
>
> Java offers the creation of direct byte buffers which are allocated outside
> of the heap.
> They need to be manually free'd, which can be accomplished using an
> documented {{clean}} method.
> The feature will be optional. After implementing, we can benchmark for
> differences in speed and garbage collection observances.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
