if message length over Integer.MAX_VALUE, the message.getBody().readAllBytes() method will throw OutOfMemoryError. But we can throw this Error in advance. It can reduce message reading.

message.getBody().readAllBytes() method achieve: when reading bytes over Integer.MAX_VALUE, it will throw OutOfMemoryError. Message is Required array size too large.

Comment From: pivotal-cla

@TungSword Please sign the Contributor License Agreement!

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

See the FAQ for frequently asked questions.

Comment From: pivotal-cla

@TungSword Thank you for signing the Contributor License Agreement!

Comment From: snicoll

Thanks for the PR but that's not what the current code does. It attempts to read the byte array using the length if possible for performance reason. If that's not possible then it calls another method that's less performant. Your change would mean that if the size is above 2GB it'd fail immediately. Also, user code should never throw OutOfMemoryError as you suggest.

Comment From: TungSword

above

But, message.getBody().readAllBytes() method will throw OutOfMemoryError. If byte array length is above 2G. The exception is "Exception in thread "main" java.lang.OutOfMemoryError: Requested array size exceeds VM limit". When we check array length before, we can aboid this method reading data. It's can find error more efficient.

Comment From: snicoll

But, message.getBody().readAllBytes() method will throw OutOfMemoryError. If byte array length is above 2G

Why? I think you're conflating the capacity of an int, and the ability to read data over 2GB. You can see that the length is a long and, while it should be very rare, a JVM (64bits that is) can read byte array over 2GB just fine as long as it has enough memory allocated.

Comment From: TungSword

But, message.getBody().readAllBytes() method will throw OutOfMemoryError. If byte array length is above 2G

Why? I think you're conflating the capacity of an int, and the ability to read data over 2GB. You can see that the length is a long and, while it should be very rare, a JVM (64bits that is) can read byte array over 2GB just fine as long as it has enough memory allocated.

I view the InputStream.java https://github.com/openjdk/jdk21/blob/master/src/java.base/share/classes/java/io/InputStream.java class, inside this class readAllBytes() method will call readNBytes() method. It judge in line 419 - 421. if (MAX_BUFFER_SIZE - total < nread) { throw new OutOfMemoryError("Required array size too large"); } It will throw OutOfMemoryError Exception.

Comment From: TungSword

But, message.getBody().readAllBytes() method will throw OutOfMemoryError. If byte array length is above 2G

Why? I think you're conflating the capacity of an int, and the ability to read data over 2GB. You can see that the length is a long and, while it should be very rare, a JVM (64bits that is) can read byte array over 2GB just fine as long as it has enough memory allocated.

I view the InputStream.java https://github.com/openjdk/jdk21/blob/master/src/java.base/share/classes/java/io/InputStream.java class, inside this class readAllBytes() method will call readNBytes() method. It judge in line 419 - 421. if (MAX_BUFFER_SIZE - total < nread) { throw new OutOfMemoryError("Required array size too large"); } It will throw OutOfMemoryError Exception.

When InputStream throw OutOfMemoryError Exception. The Programe will spend many time to read 2G data. It will affect interface performance.

In my test, I upload a file over 2G. It will convert to byte array. like this @PostMapping("/testbyte") public String testByte(@RequestBody byte[] bytes){ System.out.println(bytes); return "test byte success."; } The Error info: java.lang.OutOfMemoryError: Required array size too large at java.base/java.io.InputStream.readNBytes(InputStream.java:417) ~[na:na] at java.base/java.io.InputStream.readAllBytes(InputStream.java:346) ~[na:na] at org.springframework.http.converter.ByteArrayHttpMessageConverter.readInternal(ByteArrayHttpMessageConverter.java:57) ~[spring-web-6.1.14.jar:6.1.14] at org.springframework.http.converter.ByteArrayHttpMessageConverter.readInternal(ByteArrayHttpMessageConverter.java:38) ~[spring-web-6.1.14.jar:6.1.14] at org.springframework.http.converter.AbstractHttpMessageConverter.read(AbstractHttpMessageConverter.java:198) ~[spring-web-6.1.14.jar:6.1.14]

Comment From: bclozel

I still think we should decline this PR for the following reasons:

  1. this completely removes the readNBytes variant where we use the known content length in advance to optimize the allocation.
  2. In the JDK MAX_BUFFER_SIZE isn't Integer.MAX_VALUE but actually Integer.MAX_VALUE - 8. So Spring would throw the exception in some cases, and the JDK would in some others
  3. OutOfMemoryError is typically thrown by the code in charge of memory allocation, which is not Spring

Overall I think we would somehow optimize for a rare case, make maintenance more complicated and implement an inconsistent and surprising behavior.

Comment From: TungSword

I still think we should decline this PR for the following reasons:

  1. this completely removes the readNBytes variant where we use the known content length in advance to optimize the allocation.
  2. In the JDK MAX_BUFFER_SIZE isn't Integer.MAX_VALUE but actually Integer.MAX_VALUE - 8. So Spring would throw the exception in some cases, and the JDK would in some others
  3. OutOfMemoryError is typically thrown by the code in charge of memory allocation, which is not Spring

Overall I think we would somehow optimize for a rare case, make maintenance more complicated and implement an inconsistent and surprising behavior.

OK. I got it. Thank you.