Derby
  1. Derby
  2. DERBY-4686

SQLBinary.writeBlob is inefficient, reading one byte at a time from the source BLOB

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.7.1.1
    • Fix Version/s: 10.5.3.1, 10.6.2.1, 10.7.1.1
    • Component/s: Store
    • Labels:
    • Issue & fix info:
      Known fix, Newcomer
    • Bug behavior facts:
      Performance

      Description

      SQLBinary.writeBlob is inefficient, since it is only reading one byte at the time from the source BLOB.
      It would be better if a transfer buffer was used to facilitate the write.

      1. DERBY-4686-2.diff
        0.6 kB
        Yun Lee
      2. DERBY-4686-1.diff
        2 kB
        Yun Lee
      3. DERBY-4686-1.diff
        2 kB
        Kristian Waagan
      4. DERBY-4686.diff
        1 kB
        Yun Lee

        Issue Links

          Activity

          Gavin made changes -
          Workflow jira [ 12512325 ] Default workflow, editable Closed status [ 12799743 ]
          Rick Hillegas made changes -
          Affects Version/s 10.7.1.1 [ 12315564 ]
          Affects Version/s 10.7.1.0 [ 12314971 ]
          Fix Version/s 10.7.1.1 [ 12315564 ]
          Fix Version/s 10.7.1.0 [ 12314971 ]
          Knut Anders Hatlen made changes -
          Fix Version/s 10.6.2.1 [ 12315343 ]
          Fix Version/s 10.6.2.0 [ 12315342 ]
          Kathey Marsden made changes -
          Fix Version/s 10.6.2.0 [ 12315342 ]
          Fix Version/s 10.6.1.1 [ 12314973 ]
          Myrna van Lunteren made changes -
          Link This issue is part of DERBY-4609 [ DERBY-4609 ]
          Kristian Waagan made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Kristian Waagan added a comment -

          Closing issue.

          Show
          Kristian Waagan added a comment - Closing issue.
          Kristian Waagan made changes -
          Fix Version/s 10.5.3.1 [ 12314182 ]
          Fix Version/s 10.6.1.1 [ 12314973 ]
          Hide
          Kristian Waagan added a comment -

          Backported to 10.6 with revision 957472 and to 10.5 with revision 957473.

          Show
          Kristian Waagan added a comment - Backported to 10.6 with revision 957472 and to 10.5 with revision 957473.
          Hide
          Kristian Waagan added a comment -

          Hi Yun,

          It was Knut Anders who suggested the buffer size optimization, but in any case, you're welcome
          If possible, we also try to keep lines shorter than 80 characters, but that's a detail.

          I committed patch DERBY-4686-2.diff to trunk with revision 955634.

          Thanks,

          Show
          Kristian Waagan added a comment - Hi Yun, It was Knut Anders who suggested the buffer size optimization, but in any case, you're welcome If possible, we also try to keep lines shorter than 80 characters, but that's a detail. I committed patch DERBY-4686 -2.diff to trunk with revision 955634. Thanks,
          Yun Lee made changes -
          Attachment DERBY-4686-2.diff [ 12447341 ]
          Hide
          Yun Lee added a comment -

          Hi, Kristian. Thanks for your scrupulous help, I'm excited! I have added a buffer size optimization in DERBY-4686-2.diff, please check it.

          And I will be careful for tab policy in my future work, thanks for your correction!

          Show
          Yun Lee added a comment - Hi, Kristian. Thanks for your scrupulous help, I'm excited! I have added a buffer size optimization in DERBY-4686 -2.diff, please check it. And I will be careful for tab policy in my future work, thanks for your correction!
          Hide
          Knut Anders Hatlen added a comment -

          Would it make sense to replace this line

          byte[] buffer = new byte[LEN_OF_BUFFER_TO_WRITE_BLOB];

          with this

          byte[] buffer = new byte[Math.min(len, LEN_OF_BUFFER_TO_WRITE_BLOB)];

          so that we don't allocate a buffer that's larger than the value we're going to read?

          Show
          Knut Anders Hatlen added a comment - Would it make sense to replace this line byte[] buffer = new byte [LEN_OF_BUFFER_TO_WRITE_BLOB] ; with this byte[] buffer = new byte [Math.min(len, LEN_OF_BUFFER_TO_WRITE_BLOB)] ; so that we don't allocate a buffer that's larger than the value we're going to read?
          Kristian Waagan made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Fix Version/s 10.7.0.0 [ 12314971 ]
          Resolution Fixed [ 1 ]
          Hide
          Kristian Waagan added a comment -

          All tests passed.
          Committed fix to trunk with revision 954748.

          Thank you, Yun!

          Show
          Kristian Waagan added a comment - All tests passed. Committed fix to trunk with revision 954748. Thank you, Yun!
          Kristian Waagan made changes -
          Attachment DERBY-4686-1.diff [ 12447113 ]
          Hide
          Kristian Waagan added a comment -

          Thanks, Yun.

          I think the patch looks good, and once the final test run has completed I intend to commit it.
          The latest patch contained a mix of tab and space indents, so I replaced all tabs with four spaces and attached the updated patch. I also verified that the changed code is indeed exercised in our test suite (I found UserLobTest and BooleanValuesTest).

          Show
          Kristian Waagan added a comment - Thanks, Yun. I think the patch looks good, and once the final test run has completed I intend to commit it. The latest patch contained a mix of tab and space indents, so I replaced all tabs with four spaces and attached the updated patch. I also verified that the changed code is indeed exercised in our test suite (I found UserLobTest and BooleanValuesTest).
          Yun Lee made changes -
          Attachment DERBY-4686-1.diff [ 12446948 ]
          Hide
          Yun Lee added a comment -

          Kristian, thanks for your advice. I agree with your opinion and have attached a new patch, please check it, thanks!

          Show
          Yun Lee added a comment - Kristian, thanks for your advice. I agree with your opinion and have attached a new patch, please check it, thanks!
          Hide
          Kristian Waagan added a comment -

          Thanks for the patch, Yun

          The patch looks good. I think we can improve it by verifying that we actually transfer len bytes, and to avoid infinite loop in the case where there are fewer than len bytes in the stream.
          We can probably reuse error msg SQLState.SET_STREAM_INEXACT_LENGTH_DATA (see ReaderToUTF8Stream.checkSufficientData).

          Regards,

          Show
          Kristian Waagan added a comment - Thanks for the patch, Yun The patch looks good. I think we can improve it by verifying that we actually transfer len bytes, and to avoid infinite loop in the case where there are fewer than len bytes in the stream. We can probably reuse error msg SQLState.SET_STREAM_INEXACT_LENGTH_DATA (see ReaderToUTF8Stream.checkSufficientData). Regards,
          Yun Lee made changes -
          Attachment DERBY-4686.diff [ 12446750 ]
          Hide
          Yun Lee added a comment -

          In the patch, I have added a buffer in transfering, please check it, thanks!

          Show
          Yun Lee added a comment - In the patch, I have added a buffer in transfering, please check it, thanks!
          Yun Lee made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Yun Lee made changes -
          Field Original Value New Value
          Assignee Yun Lee [ yunlee ]
          Kristian Waagan created issue -

            People

            • Assignee:
              Yun Lee
              Reporter:
              Kristian Waagan
            • Votes:
              1 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development