Uploaded image for project: 'Couchbase Server'
  1. Couchbase Server
  2. MB-49284

Performance: Plasma constructors shallow copy huge Config object many times

    XMLWordPrintable

Details

    • Untriaged
    • 1
    • Unknown

    Description

      plasma.go constructors New(), New2(), New3() all accept argument cfg Config, which is a struct, therefore it is being passed by value not by reference. This means each call, and each child this is passed to that does the same thing, makes a new shallow copy of the entire Config struct. By my count this struct currently contains 113 fields, so these copies are doing a lot of extra CPU work and consuming significant stack.

      E.g. a call to New(cfg) triggers the following copies (at least the first 5, and possibly up to 7, assuming none of the great-grandchildren copy it further):

      1. New(cfg) – arg copied on call
      2. New2(cfg, ...) – copied to child call
      3. New3(cfg, ...) – copied to grandchild call
      4. applyConfigDefaults(cfg, ...) – copied to first great-grandchild call
      5. cfg = applyConfigDefaults(...) – and copied again assigning the returned copy back to New3's cfg variable
      6. doRecovery(..., cfg, ...) – copied to second great-grandchild if this call is made
      7. newPlasmaSkeleton(cfg, ...) – copied to third great-grandchild if this call is made

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          jliang John Liang added a comment -

          This is only called once per instance created. It does not happen very often. Having said that, we could pass in pointer and do the copy until the very last one. Not sure how much CPU it will actually saved though.

          jliang John Liang added a comment - This is only called once per instance created. It does not happen very often. Having said that, we could pass in pointer and do the copy until the very last one. Not sure how much CPU it will actually saved though.

          Build couchbase-server-7.1.0-1941 contains plasma commit 02cb58c with commit message:
          MB-49284 Send Config by reference (merge conflict fix)

          build-team Couchbase Build Team added a comment - Build couchbase-server-7.1.0-1941 contains plasma commit 02cb58c with commit message: MB-49284 Send Config by reference (merge conflict fix)

          Build couchbase-server-7.1.0-1941 contains plasma commit d7f6e60 with commit message:
          MB-49284 Send Config by reference

          build-team Couchbase Build Team added a comment - Build couchbase-server-7.1.0-1941 contains plasma commit d7f6e60 with commit message: MB-49284 Send Config by reference

          Fix is cleanup. No extra test needed.

          srinath.duvuru Srinath Duvuru added a comment - Fix is cleanup. No extra test needed.

          People

            Dmitriy.Kalugin-Balashov Dmitriy Kalugin-Balashov
            kevin.cherkauer Kevin Cherkauer (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes

                PagerDuty