SPR-15762: improve performance of ContentCachingRequestWrapper

[xnslong]

The ContentCachingInputStream class has only implemented the int read() method of the stream, let the corresponding read(byte[], int, int) method inherit from the InputStream class.

However, reading a few bytes from an InputStream with a loop call of the int read() method and a call with the read(byte[], int, int) method differs a lot. The latter method is much faster than the former method. My code and result for this comparison are listed bellow.

byte[] bytes = new byte[100_000]; // 100k
// test reading an input stream with loop method
ByteArrayInputStream in1 = new ByteArrayInputStream(bytes);
long start = System.nanoTime();
for (int i = 0; i < 100_000; i++) {
    in1.read();
}
System.out.println("loop calling cost: " + (System.nanoTime() - start) + "ns");

// test reading the same input stream with batch method
ByteArrayInputStream in2 = new ByteArrayInputStream(bytes);
byte[] buffer = new byte[1000];
int c = 0;
start = System.nanoTime();
while (c < 100_000) {
    c += in2.read(buffer);
}
System.out.println("batch calling cost: " + (System.nanoTime() - start) + "ns");

Finally, I got the following result on my machine.

loop calling cost: 9158452ns
batch calling cost: 85389ns

[pivotal-issuemaster]

@xnslong Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@xnslong Thank you for signing the Contributor License Agreement!

[snicoll]

@xnslong can you please create a Jira issue for this change?

[xnslong]

@snicoll I have created a jira issue SPR-15762. How can I associate this PR to that issue?

[snicoll]

I've done that for you, check the update on the Jira issue

[xnslong]

@snicoll Sorry for the late reply, I checked the benchmark on the issue, and I found there is mistake in my code. After it is fixed, it performs very differently. Would you please take a look?

[xnslong]

My mistake is fixed now

[bclozel]

Merged with 4d0800f
Thanks a lot!

[xnslong]

Thanks! @bclozel

I checked the merge commit in last comment, but I found some code may cause problem. Show as the following

@Override
 public int read(final byte[] b) throws IOException {
     int count = super.read(b);
     // the following code may cause problem
     if (!this.overflow && count > 0) {
         if (contentCacheLimit != null && cachedContent.size() == contentCacheLimit) {
             this.overflow = true;
             handleContentOverflow(contentCacheLimit);
         } else {
               int sizeToCache = contentCacheLimit == null || count + cachedContent.size() < CacheLimit
                       ? count
                       : contentCacheLimit - cachedContent.size();
               cachedContent.write(b, 0, sizeToCache);
               if (sizeToCache < count) {
                 this.overflow = true;
                 handleContentOverflow(contentCacheLimit);
             }
         }
     }
     return count;
 }

The super.read(byte[] b) method will finally call the int read(b, off, len) method to read data. And the read(b, off, len) method has cached the contents already. So the overridden read(byte[] b) method will cause the content to be duplicated in the cache.

I think it may be good not to override this method, because the read(b, off, len) method will perform well, and the super.read(byte[] b) is not costly.