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

Lars Hofhansl commented on HBASE-10015:
---------------------------------------

I think it would be hard to
# push reader switching to the scanning thread
# guarantee that the readers will eventually be switched to the compaction can 
proceed and finished

What if a scanner isn't closed? Or nobody ever calls next() on it? Then we have 
to wait for the lease to expire before the compaction can finish.

next()/seek()/reseek() are not as critical as they are not called remotely as 
often as peek. peek() is also a very small method and (according to our perf 
expert here) small synchronized methods have the greatest potential to throw 
the branch prediction off.


> Major performance improvement: Avoid synchronization in StoreScanner
> --------------------------------------------------------------------
>
>                 Key: HBASE-10015
>                 URL: https://issues.apache.org/jira/browse/HBASE-10015
>             Project: HBase
>          Issue Type: Bug
>            Reporter: Lars Hofhansl
>            Assignee: Lars Hofhansl
>             Fix For: 0.98.0, 0.96.1, 0.94.15
>
>         Attachments: 10015-0.94-v2.txt, 10015-0.94-v3.txt, 
> 10015-0.94-withtest.txt, 10015-0.94.txt, 10015-trunk-v2.txt, 
> 10015-trunk-v3.txt, 10015-trunk.txt, TestLoad.java
>
>
> Did some more profiling (this time with a sampling profiler) and 
> StoreScanner.peek() showed up a lot in the samples. At first that was 
> surprising, but peek is synchronized, so it seems a lot of the sync'ing cost 
> is eaten there.
> It seems the only reason we have to synchronize all these methods is because 
> a concurrent flush or compaction can change the scanner stack, other than 
> that only a single thread should access a StoreScanner at any given time.
> So replaced updateReaders() with some code that just indicates to the scanner 
> that the readers should be updated and then make it the using thread's 
> responsibility to do the work.
> The perf improvement from this is staggering. I am seeing somewhere around 3x 
> scan performance improvement across all scenarios.
> Now, the hard part is to reason about whether this is 100% correct. I ran 
> TestAtomicOperation and TestAcidGuarantees a few times in a loop, all still 
> pass.
> Will attach a sample patch.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Reply via email to