[
https://issues.apache.org/jira/browse/HBASE-14921?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15223005#comment-15223005
]
Anastasia Braginsky commented on HBASE-14921:
---------------------------------------------
{quote}
CellBlock createCellBlocks(Comparator<? super Cell> comparator, int min, int
max, boolean descending);
This is for making a sub set of original block. Can we have a better name pls?
{quote}
yes, I’ll change to createSubCellMap()
bq. getCellFromIndex(int i) -> getCell (int index) Is this enough?
yes, changed already
bq. getValidIndex -> Name looks diff to understand what it does. Pls add some
comments.. Other places wherever necessary
added comments
bq. HeapMemStoreLAB memStoreLAB -> WHy need the impl type here? We have MSLAB
interface now.
Unfortunately, Chunk is internal class of HeapMemStoreLAB, therefore interface
MemStoreLAB can not help. The best solution is to move the Chunk outside of the
HeapMemStoreLAB, but I didn’t want to do this refactoring as part of this JIRA.
bq. HeapMemStoreLAB.Chunk[] chunks -> Do we need to keep Chunk refs? byte[]
refs is enough? Or is this for return back to MSLAB pool after the DISK flush?
This point was already discussed, but generally for off-heaping and re-usage.
bq. Why this move of init to here immediately after the new Chunk is made? When
multiple threads are at this point, there is a chance that more than one will
do this init call make make byte[]. If it is after compareAndSet, it is sure
abt the concurrency solution.
First, this is for the case when ChunkPool is null and actually the entire code
(as it is written now) doesn’t work if ChunkPool is null. The issue of
ensuring that ChunkPool is never null was already discussed here.
Second, if multiple threads run this code each thread is going to allocate and
initiate its own chunk.
bq. allocateChunk - Why is it needed when we have getOrMakeChunk? Who will
guarantee thread safety here?
allocateChunk() is a method of MemStoreChunkPool class and getOrMakeChunk() is
a method of HeapMemStoreLAB class, so they do not intersect in their
responsibilities. allocateChunk() is safe for multi-threading because it uses
AtomicInteger, new-based allocation, ConcurrentHashMap, and finally each thread
is going to initialize its own chunk (it is not shared memory yet).
{quote}
//org.junit.Assert.assertTrue("\n\n inside chunk initialization 3", false);
pls remove commented code.. Some other cases also.
{quote}
surely will remove
{quote}
-reclaimedChunks.add(chunk);
I can not see where we will be adding it as per the patch? The initial created
chunks were added here.
{quote}
It was my bug, which is already found and already fixed. The call for
reclaimedChunks.add(chunk) on pre-allocation is indeed missing here.
bq. Smaller than 256 bytes? Yes. How'd you get 256 bytes? I didn't follow.
Sorry.
Just guessed that number for easier calculation.
> Memory optimizations
> --------------------
>
> Key: HBASE-14921
> URL: https://issues.apache.org/jira/browse/HBASE-14921
> Project: HBase
> Issue Type: Sub-task
> Affects Versions: 2.0.0
> Reporter: Eshcar Hillel
> Assignee: Anastasia Braginsky
> Attachments: CellBlocksSegmentInMemStore.pdf,
> CellBlocksSegmentinthecontextofMemStore(1).pdf, HBASE-14921-V01.patch,
> HBASE-14921-V02.patch
>
>
> Memory optimizations including compressed format representation and offheap
> allocations
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
