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

          Hide
          Kristian Waagan added a comment -

          Closing issue.

          Show
          Kristian Waagan added a comment - Closing issue.
          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,
          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?
          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!
          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).
          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,
          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!

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development