Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0
    • Component/s: None
    • Labels:
      None

      Description

      Right now log record serialization is synchronized. We can improve concurrency by serializing to a ram buffer outside synchronization. The cost will be RAM usage for buffering, and more complex concurrency in the tlog itself (i.e. we must ensure that a close does not happen without flushing all in-RAM buffers)

      1. SOLR-3715.patch
        18 kB
        Yonik Seeley

        Activity

        Hide
        Yonik Seeley added a comment -

        Here's a patch that moves log record serialization outside of the synchronized block.

        All tests pass, but I could detect no meaningful speedup on my system.
        After further investigation, I believe this is due to the fact that our binary (javabin) serialization is really fast. I did some profiling and it was between .25% (sampling) and 2% (instrumentation) of the total runtime. Removing synchronization around such a small percent of time is not going to show up much on my 4 CPU box.

        I still plan to commit this as it still represents an architectural improvement, may be more mesurable in high-core systems, and is a stepping stone toward future improvements.

        Show
        Yonik Seeley added a comment - Here's a patch that moves log record serialization outside of the synchronized block. All tests pass, but I could detect no meaningful speedup on my system. After further investigation, I believe this is due to the fact that our binary (javabin) serialization is really fast. I did some profiling and it was between .25% (sampling) and 2% (instrumentation) of the total runtime. Removing synchronization around such a small percent of time is not going to show up much on my 4 CPU box. I still plan to commit this as it still represents an architectural improvement, may be more mesurable in high-core systems, and is a stepping stone toward future improvements.
        Hide
        Yonik Seeley added a comment -

        It looks like there's a bug lurking here.... I'm looking into it.
        http://find.searchhub.org/document/7510cdb2423bcf7d

        Show
        Yonik Seeley added a comment - It looks like there's a bug lurking here.... I'm looking into it. http://find.searchhub.org/document/7510cdb2423bcf7d
        Hide
        Yonik Seeley added a comment -

        I just committed the fix for the bug introduced by this issue.
        http://svn.apache.org/viewvc?view=revision&revision=1374480

        Show
        Yonik Seeley added a comment - I just committed the fix for the bug introduced by this issue. http://svn.apache.org/viewvc?view=revision&revision=1374480
        Hide
        Uwe Schindler added a comment -

        Closed after release.

        Show
        Uwe Schindler added a comment - Closed after release.

          People

          • Assignee:
            Yonik Seeley
            Reporter:
            Yonik Seeley
          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development