FAQ
Lars Hofhansl created HBASE-10015:
-------------------------------------

              Summary: 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
          Attachments: 10015-0.94.txt

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)

Search Discussions

  • Lars Hofhansl (JIRA) at Nov 20, 2013 at 8:12 pm
    [ https://issues.apache.org/jira/browse/HBASE-10015?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

    Lars Hofhansl updated HBASE-10015:
    ----------------------------------

         Attachment: 10015-0.94.txt

    Sample 0.94 patch.
    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
    Attachments: 10015-0.94.txt


    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)
  • Lars Hofhansl (JIRA) at Nov 20, 2013 at 8:24 pm
    [ https://issues.apache.org/jira/browse/HBASE-10015?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13828074#comment-13828074 ]

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

    The 3x improvement I see for tall tables. For wider tables the improvement is less pronounced (for 5 columns I see a 20% or so improvement).

    Also a note as to why I think this should be correct: Even with the current synchronized, a next/peek/reseek that has already started, will finish with the old reader. That has not changed.

    Need to look closer at races between close() and next/peek/reseek, as close could be called from another thread (I think), when the lease expired.
    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
    Attachments: 10015-0.94.txt


    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)
  • Lars Hofhansl (JIRA) at Nov 20, 2013 at 8:32 pm
    [ https://issues.apache.org/jira/browse/HBASE-10015?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13828082#comment-13828082 ]

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

    Actually the main difference is between ScanWildcardColumnTracker and ExplicitColumnTracker. With ExplicitColumnTracker there are still many reseeks that outweigh the performance improvement seen here.
    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
    Attachments: 10015-0.94.txt


    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)
  • Vladimir Rodionov (JIRA) at Nov 20, 2013 at 8:40 pm
    [ https://issues.apache.org/jira/browse/HBASE-10015?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13828088#comment-13828088 ]

    Vladimir Rodionov commented on HBASE-10015:
    -------------------------------------------

    It is interesting. Java synchronization w/o thread contention cost is close to zero. You would see the difference only when you run multiple threads accessing the same StoreScanner.
    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
    Attachments: 10015-0.94.txt


    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)
  • Lars Hofhansl (JIRA) at Nov 20, 2013 at 8:56 pm
    [ https://issues.apache.org/jira/browse/HBASE-10015?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13828103#comment-13828103 ]

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

    That is true as far as actual thread synchronization goes.

    Every synchronize still places a read and write memory fence, though, and I think that is the effect we're seeing. I was a bit surprised about the magnitude of this myself.

    I can try switching one of my cores off and measure this again, if the issues is memory fencing we should not see any improvement with one core only.

    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
    Attachments: 10015-0.94.txt


    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)

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupissues @
categorieshbase, hadoop
postedNov 20, '13 at 8:12p
activeNov 20, '13 at 8:56p
posts6
users1
websitehbase.apache.org

1 user in discussion

Lars Hofhansl (JIRA): 6 posts

People

Translate

site design / logo © 2021 Grokbase