[jira] Created: (DERBY-3768) Make EmbedBlob.length use skip instead of read

Make EmbedBlob.length use skip instead of read
----------------------------------------------

                 Key: DERBY-3768
                 URL: https://issues.apache.org/jira/browse/DERBY-3768
             Project: Derby
          Issue Type: Improvement
          Components: JDBC
    Affects Versions: 10.5.0.0
            Reporter: Kristian Waagan
            Assignee: Kristian Waagan
            Priority: Minor
             Fix For: 10.5.0.0


EmbedBlob.length uses read to process the whole Blob when the length has not been encoded at the head of the stream.
Using skip instead of read can lead to better performance.

I also plan to make some minor cleanups under this issue; JavaDoc and to rename a variable.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-3768?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan updated DERBY-3768:
-----------------------------------

    Attachment: derby-3768-1a-length_skip.diff

'derby-3768-1a-length_skip.diff' replaces the read loop with a skip loop to determine the stream length. Also removes unused imports.

Patch ready for review.

> Make EmbedBlob.length use skip instead of read
> ----------------------------------------------
>
>                 Key: DERBY-3768
>                 URL: https://issues.apache.org/jira/browse/DERBY-3768
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC
>    Affects Versions: 10.5.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>            Priority: Minor
>             Fix For: 10.5.0.0
>
>         Attachments: derby-3768-1a-length_skip.diff
>
>
> EmbedBlob.length uses read to process the whole Blob when the length has not been encoded at the head of the stream.
> Using skip instead of read can lead to better performance.
> I also plan to make some minor cleanups under this issue; JavaDoc and to rename a variable.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3768?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612062#action_12612062 ]

Kristian Waagan commented on DERBY-3768:
----------------------------------------

suites.All and derbyall ran without failures with Solaris 10 and Sun JDK 1.6.

> Make EmbedBlob.length use skip instead of read
> ----------------------------------------------
>
>                 Key: DERBY-3768
>                 URL: https://issues.apache.org/jira/browse/DERBY-3768
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC
>    Affects Versions: 10.5.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>            Priority: Minor
>             Fix For: 10.5.0.0
>
>         Attachments: derby-3768-1a-length_skip.diff
>
>
> EmbedBlob.length uses read to process the whole Blob when the length has not been encoded at the head of the stream.
> Using skip instead of read can lead to better performance.
> I also plan to make some minor cleanups under this issue; JavaDoc and to rename a variable.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3768?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612066#action_12612066 ]

Dag H. Wanvik commented on DERBY-3768:
--------------------------------------

Patch look good to me. I only did code inspection, since I am not sure which test
best exercises this code.

The skip semantics in Java input streams is a bit broken (it may return 0 bytes
skipped more or less at will and still be compliant,
cf. http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long)),
but I see you have coded it safely here, maybe it would be a good idea
to make a utility method for skipping input streams ("skipFully").

+1

> Make EmbedBlob.length use skip instead of read
> ----------------------------------------------
>
>                 Key: DERBY-3768
>                 URL: https://issues.apache.org/jira/browse/DERBY-3768
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC
>    Affects Versions: 10.5.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>            Priority: Minor
>             Fix For: 10.5.0.0
>
>         Attachments: derby-3768-1a-length_skip.diff
>
>
> EmbedBlob.length uses read to process the whole Blob when the length has not been encoded at the head of the stream.
> Using skip instead of read can lead to better performance.
> I also plan to make some minor cleanups under this issue; JavaDoc and to rename a variable.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-3768?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan updated DERBY-3768:
-----------------------------------

    Attachment: derby-3768-1b-length_skip.diff

Committed revision 1b of the patch, in which the only change over 1a is the addition of an else-block, to trunk with revision 675169.

Thanks for the review Dag.
I agree the InputStream.skip contract is somewhat difficult. We already have a utility class for UTF8 streams (iapi.util.UTF8Util), so adding one for generic InputStreams is a good idea.
I created DERBY-3770 for this task.

> Make EmbedBlob.length use skip instead of read
> ----------------------------------------------
>
>                 Key: DERBY-3768
>                 URL: https://issues.apache.org/jira/browse/DERBY-3768
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC
>    Affects Versions: 10.5.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>            Priority: Minor
>             Fix For: 10.5.0.0
>
>         Attachments: derby-3768-1a-length_skip.diff, derby-3768-1b-length_skip.diff
>
>
> EmbedBlob.length uses read to process the whole Blob when the length has not been encoded at the head of the stream.
> Using skip instead of read can lead to better performance.
> I also plan to make some minor cleanups under this issue; JavaDoc and to rename a variable.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-3768?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan updated DERBY-3768:
-----------------------------------

    Attachment: derby-3768-2a-misc_fixes.diff

'derby-3768-2a-misc_fixes.diff' contains a set of small fixes:
 - JavaDoc improvements
 - set locator value explicitly to indicate default value (must not be a valid locator)
 - renamed 'myLength' to 'streamLength'
 - keep variables 'materialized' and 'streamLength' in sync
 - throw exception in second constructor (mistakenly swallowed)
 - only clear LOB mapping if a mapping has been created

Awaiting test run completion.
Patch ready for review.

> Make EmbedBlob.length use skip instead of read
> ----------------------------------------------
>
>                 Key: DERBY-3768
>                 URL: https://issues.apache.org/jira/browse/DERBY-3768
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC
>    Affects Versions: 10.5.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>            Priority: Minor
>             Fix For: 10.5.0.0
>
>         Attachments: derby-3768-1a-length_skip.diff, derby-3768-1b-length_skip.diff, derby-3768-2a-misc_fixes.diff
>
>
> EmbedBlob.length uses read to process the whole Blob when the length has not been encoded at the head of the stream.
> Using skip instead of read can lead to better performance.
> I also plan to make some minor cleanups under this issue; JavaDoc and to rename a variable.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3768?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613276#action_12613276 ]

Kristian Waagan commented on DERBY-3768:
----------------------------------------

suites.All and derbyall ran without failures.

> Make EmbedBlob.length use skip instead of read
> ----------------------------------------------
>
>                 Key: DERBY-3768
>                 URL: https://issues.apache.org/jira/browse/DERBY-3768
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC
>    Affects Versions: 10.5.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>            Priority: Minor
>             Fix For: 10.5.0.0
>
>         Attachments: derby-3768-1a-length_skip.diff, derby-3768-1b-length_skip.diff, derby-3768-2a-misc_fixes.diff
>
>
> EmbedBlob.length uses read to process the whole Blob when the length has not been encoded at the head of the stream.
> Using skip instead of read can lead to better performance.
> I also plan to make some minor cleanups under this issue; JavaDoc and to rename a variable.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-3768?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan updated DERBY-3768:
-----------------------------------

    Derby Info:   (was: [Patch Available])

Committed 'derby-3768-2a-misc_fixes.diff' to trunk with revision 676571.
I plan to backport these changes, and some related performance issues, to 10.4.

> Make EmbedBlob.length use skip instead of read
> ----------------------------------------------
>
>                 Key: DERBY-3768
>                 URL: https://issues.apache.org/jira/browse/DERBY-3768
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC
>    Affects Versions: 10.5.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>            Priority: Minor
>             Fix For: 10.5.0.0
>
>         Attachments: derby-3768-1a-length_skip.diff, derby-3768-1b-length_skip.diff, derby-3768-2a-misc_fixes.diff
>
>
> EmbedBlob.length uses read to process the whole Blob when the length has not been encoded at the head of the stream.
> Using skip instead of read can lead to better performance.
> I also plan to make some minor cleanups under this issue; JavaDoc and to rename a variable.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-3768?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan resolved DERBY-3768.
------------------------------------

       Resolution: Fixed
    Fix Version/s: 10.4.1.4

Backported patches 1b and 2a to 10.4 with revision 677578.

> Make EmbedBlob.length use skip instead of read
> ----------------------------------------------
>
>                 Key: DERBY-3768
>                 URL: https://issues.apache.org/jira/browse/DERBY-3768
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC
>    Affects Versions: 10.5.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>            Priority: Minor
>             Fix For: 10.4.1.4, 10.5.0.0
>
>         Attachments: derby-3768-1a-length_skip.diff, derby-3768-1b-length_skip.diff, derby-3768-2a-misc_fixes.diff
>
>
> EmbedBlob.length uses read to process the whole Blob when the length has not been encoded at the head of the stream.
> Using skip instead of read can lead to better performance.
> I also plan to make some minor cleanups under this issue; JavaDoc and to rename a variable.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-3768?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan closed DERBY-3768.
----------------------------------

> Make EmbedBlob.length use skip instead of read
> ----------------------------------------------
>
>                 Key: DERBY-3768
>                 URL: https://issues.apache.org/jira/browse/DERBY-3768
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC
>    Affects Versions: 10.5.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>            Priority: Minor
>             Fix For: 10.4.1.4, 10.5.0.0
>
>         Attachments: derby-3768-1a-length_skip.diff, derby-3768-1b-length_skip.diff, derby-3768-2a-misc_fixes.diff
>
>
> EmbedBlob.length uses read to process the whole Blob when the length has not been encoded at the head of the stream.
> Using skip instead of read can lead to better performance.
> I also plan to make some minor cleanups under this issue; JavaDoc and to rename a variable.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.