Make EmbedBlob.length use skip instead of read

• Key: DERBY-3768
• Type: Improvement
• Status: Closed
• Resolution: Fixed
• Priority: Minor
• Assignee: Kristian Waagan
• Reporter: Kristian Waagan
• Component/s: JDBC
• Affects Version/s: 10.5.1.1
• Fix Version/s: 10.4.2.0, 10.5.1.1

Description

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.

Comments

[Kristian Waagan]

'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.

[Kristian Waagan]

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

[Dag H. Wanvik]

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

[Kristian Waagan]

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.

[Kristian Waagan]

'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.

[Kristian Waagan]

suites.All and derbyall ran without failures.

[Kristian Waagan]

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.

[Kristian Waagan]

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