Kafka
  1. Kafka
  2. KAFKA-456

ProducerSendThread calls ListBuffer.size a whole bunch. That is a O(n) operation

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 0.8.0
    • Component/s: core
    • Labels:
    • Environment:
      NA

      Description

      Hi all,

      So there are various statements throughout the async code that call 'events.size', mostly for debugging purposes.
      Problem is that this call is O, so it could add up if the batch size is high. (it's a ListBuffer)

      I see this in at least ProducerSendThread (x4), likely more. Will factor this out myself soon when I start hacking on the project, just wanted to put this somewhere.

      1. KAFKA-456.patch
        0.8 kB
        David Arthur

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        55d 14h 20m 1 David Arthur 04/Oct/12 14:02
        Patch Available Patch Available Resolved Resolved
        4h 29m 1 Jun Rao 04/Oct/12 18:32
        Resolved Resolved Closed Closed
        622d 11h 39m 1 Neha Narkhede 19/Jun/14 06:11
        Tony Stevenson made changes -
        Workflow Apache Kafka Workflow [ 13051092 ] no-reopen-closed, patch-avail [ 13053511 ]
        Tony Stevenson made changes -
        Workflow no-reopen-closed, patch-avail [ 12720502 ] Apache Kafka Workflow [ 13051092 ]
        Neha Narkhede made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Jay Kreps added a comment -

        Why not just make this an ArrayBuffer so that there are 0 O operations? I suspect the ListBuffer thing was just a misunderstanding of scala collections underlying implementation (i.e. ListBuffer != ArrayList).

        Show
        Jay Kreps added a comment - Why not just make this an ArrayBuffer so that there are 0 O operations? I suspect the ListBuffer thing was just a misunderstanding of scala collections underlying implementation (i.e. ListBuffer != ArrayList).
        Jun Rao made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Assignee David Arthur [ mumrah ]
        Resolution Fixed [ 1 ]
        Hide
        Jun Rao added a comment -

        Thanks for the patch. Committed to 0.8.

        This performance issue seems to have been fixed in scala 2.9.1.
        https://issues.scala-lang.org/browse/SI-4933

        Show
        Jun Rao added a comment - Thanks for the patch. Committed to 0.8. This performance issue seems to have been fixed in scala 2.9.1. https://issues.scala-lang.org/browse/SI-4933
        David Arthur made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Affects Version/s 0.7.1 [ 12319140 ]
        Affects Version/s 0.7.2 [ 12322475 ]
        Hide
        David Arthur added a comment -

        Only compatible with 0.8

        Show
        David Arthur added a comment - Only compatible with 0.8
        David Arthur made changes -
        Field Original Value New Value
        Attachment KAFKA-456.patch [ 12547723 ]
        Hide
        David Arthur added a comment -

        I looked around in other parts of the code for similar situations (lots of calls to ListBuffer#size), didn't see any that were unnecessary.

        In the worst case, this saves two calls to size(); in the common case, it saves one call.

        Show
        David Arthur added a comment - I looked around in other parts of the code for similar situations (lots of calls to ListBuffer#size), didn't see any that were unnecessary. In the worst case, this saves two calls to size(); in the common case, it saves one call.
        Matthew Rathbone created issue -

          People

          • Assignee:
            David Arthur
            Reporter:
            Matthew Rathbone
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 2h
              2h
              Remaining:
              Remaining Estimate - 2h
              2h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development