Hello, ClassReader.readStream() can be reworked to improve its performance and reduce memory consumption. Currently there are two major problems about it:

  • we always allocate a buffer of the same size which is often several times bigger than actual amount of data being read from InputStream
  • we always return of copy of byte[] underlying in ByteArrayOutputStream

Using InputStream.available() we can pre-size the buffer in vast majority of cases and even when the whole data is not read within one iteration (for large InputStreams) we allocate as much memory as required.

Another optimization is about checking whether it was only one read from InputStream, and in this case the buffer itself can be returned instead of calling ByteArrayOutputStream.toByteArray().

I've used this benchmark available here to check impact of the patch for start-up of simple Spring Boot application:

@State(Scope.Thread)
@BenchmarkMode(Mode.SingleShotTime)
@OutputTimeUnit(TimeUnit.MILLISECONDS)
@Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
public class SpringBootApplicationBenchmark {

    private ConfigurableApplicationContext context;

    @Benchmark
    public Object startUp() {
        return context = SpringApplication.run(BenchmarkApplication.class);
    }

    @TearDown(Level.Invocation)
    public void close() {
        context.close();
    }
}

and got the following results:

original

Benchmark                                                   Mode  Cnt         Score       Error   Units
SpringBootApplicationBenchmark.startUp                        ss  200       718.760 ±    20.549   ms/op
SpringBootApplicationBenchmark.startUp:·gc.alloc.rate         ss  200        38.895 ±     0.620  MB/sec
SpringBootApplicationBenchmark.startUp:·gc.alloc.rate.norm    ss  200  49776228.720 ± 19919.815    B/op
SpringBootApplicationBenchmark.startUp:·gc.count              ss  200           ≈ 0              counts

patched

Benchmark                                                   Mode  Cnt         Score       Error   Units
SpringBootApplicationBenchmark.startUp                        ss  200       614.820 ±     7.031   ms/op
SpringBootApplicationBenchmark.startUp:·gc.alloc.rate         ss  200        41.678 ±     0.240  MB/sec
SpringBootApplicationBenchmark.startUp:·gc.alloc.rate.norm    ss  200  49003403.920 ± 21786.877    B/op
SpringBootApplicationBenchmark.startUp:·gc.count              ss  200           ≈ 0              counts

Comment From: sbrannen

The following is our general stance on modification of the repackaged ASM code.

Please refrain from modifying classes under org.springframework.asm, org.springframework.cglib, and org.springframework.objenesis. Those include repackaged forks of the third-party libraries ASM, CGLIB, and Objenesis. Any refactoring to those classes should take place upstream in the originating repository. The Spring Framework will then pick up the changes when syncing with official updates of the forked third-party libraries.

In any case, let's see if @jhoeller wishes to patch our repackaged ASM code with your proposal.

Comment From: stsypanov

@sbrannen Sorry, I totally forgot that org.springframework.asm is 3rd-party code

Comment From: sbrannen

@sbrannen Sorry, I totally forgot that org.springframework.asm is 3rd-party code

No problem.

I was about to point you to the ASM repository, but I see that you just created this PR there 17 minutes ago. 😉

@jhoeller, in light of that we can wait on that PR to get merged into ASM proper and then consume it later.

Comment From: sbrannen

Closing in favor of the upstream PR against ASM: https://gitlab.ow2.org/asm/asm/-/merge_requests/314