Repository: netty/netty
Issue: File upload extremely slow

### Expected behavior

Uploading a file should be almost instantaneous in a local environment

### Actual behavior

Uploading a file is taking approximately 1 minute

### Steps to reproduce

- `./gradlew run`
- Upload a 25-30mb file (the project README has an example curl)

### Minimal yet complete reproducer code (or URL to code)
https://github.com/jameskleeh/netty-memory-leak
Checkout the `upload_perf` branch

### Netty version
This started happening in 4.1.60 and is present in 4.1.63

### JVM version (e.g. `java -version`)
JDK 8

### OS version (e.g. `uname -a`)
Darwin MacBook-Pro.local 20.3.0 Darwin Kernel Version 20.3.0: Thu Jan 21 00:07:06 PST 2021; root:xnu-7195.81.3~1/RELEASE_X86_64 x86_64

Issue comments:
There are some benchmarks within Netty that shows exactly the contrary (whatever Memory, Mixed or Disk Factory based).
The results are not using big file however, just 512KB but with more than 6 000 upload/s, so very quick, and about 3 GB/s.
I've tested by changing size to 25 MB, increasing Factory size to 1GB, and it gives 131 upload/s, so 3.2 GB/s (so less than 1s to upload).

Trace of benchmark using this 25 MB file test: (HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel is the reference in your cases)

     Benchmark                                                                           Mode  Cnt  Score   Error   Units
     HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel   thrpt    6  0,132 ± 0,005  ops/ms
     HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel   thrpt    6  0,131 ± 0,005  ops/ms
     HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel   thrpt    6  0,118 ± 0,003  ops/ms
     HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel     thrpt    6  0,131 ± 0,001  ops/ms
     HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel  thrpt    6  0,124 ± 0,004  ops/ms
     HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel  thrpt    6  0,125 ± 0,005  ops/ms
     HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel  thrpt    6  0,075 ± 0,002  ops/ms
     HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel    thrpt    6  0,125 ± 0,004  ops/ms
     
     [817.169s][info   ][gc,heap,exit ] Heap
     [817.169s][info   ][gc,heap,exit ]  garbage-first heap   total 522240K, used 69233K [0x0000000602400000, 0x0000000800000000)
     [817.169s][info   ][gc,heap,exit ]   region size 2048K, 35 young (71680K), 2 survivors (4096K)
     [817.169s][info   ][gc,heap,exit ]  Metaspace       used 17320K, capacity 17729K, committed 17792K, reserved 1064960K
     [817.169s][info   ][gc,heap,exit ]   class space    used 1790K, capacity 1906K, committed 1920K, reserved 1048576K

I also test using the Junit tests testBIgFileUploadDelimiterInMiddleChunkDecoderXxxxFactory (where Xxx can be Disk, Memory, Mixed) using a 25 MB file, and it gives less than 0.5s (even 0,05s with Memory Factory) and only 1 MB of memory allocated bu the Factory.

Maybe you are using a Factory disk based (or mixed with a lower limit than file size), such that it depends on write disk speed (therefore on your disk speed)?

Also, regarding your other issue, have you clean the factory and HttpData correctly ?

      for (InterfaceHttpData httpData: decoder.getBodyHttpDatas()) {
          httpData.release();
          factory.removeHttpDataFromClean(request, httpData);
      }
      factory.cleanAllHttpData();
      decoder.destroy();

Could you setup a simple test without your Micronaut code ?

@fredericBregier Our test cases that exercise the same code seem to perform OK as well. I've only experienced this with curl. I don't know how to setup a test given the complexity and the number of factors. Simply changing the Netty version, with the Micronaut code exactly the same, shows the issue. That leads me to believe the issue lies in Netty.
But When I did tests using either Junit or Benchmark from Netty code, it appears that it seems not an issue of Netty.
So I believe there is something wrong in the way you create the Factory or free up the factory and the decoder and the HttpData.
But as the code or Micronaut is huge, I cannot deep into it. ;-)
@fredericBregier Unfortunately if that is the approach then I believe we will be stuck on a relatively old version of Netty until we can investigate alternatives
There is also an example within Netty that starts a simple Http Web server with multipart activated.
See https://github.com/netty/netty/tree/4.1/example/src/main/java/io/netty/example/http/upload
You can use your own browser (using inspection to see the speed from browser point of view).
I've uploaded a file of 420 MB in 3.2s (131 MB/s) and I repeat the operation, with still 370 MB max memory associated with the Java Web server.

So I believe the issue is not within Netty but the way you use it, sorry.
@jameskleeh what a reproducer + run the profiler in the parallel?
Guys the fact remains that in a patch release of Netty behaviour has regressed such that we have memory leaks and performance problems which cannot be acceptable. One thing is these changes go into a new minor or major versions, but patch versions surely have to maintain behaviour and if a regression happens in a patch it needs to be taken seriously. 

Can the changes that caused this regression be rolled back if they are not production ready? Micronaut is a large user of Netty with many users and it is disheartening to see feedback addressed with "well our tests pass so sorry"
It seems as well that Vertx is having Memory leak issues in a similar area #11184
@graemerocher I agree for most of what you wrote, but where is the reproducer?

I cannot go deep within either Micronaut or Vertx. I have to have a reproducer to understand.

Note that the previous version (58 or before) was leaking a lot (memory, speed, wrong assignment with loss of data, unkown from end user). And we put much effort to add as much as possible Junit tests (or benchmarks) to ensure nothing is broken and even that those issues (memoey, speed, wrong assignment with loss of data, unkown from end user) were fixed.

If this so reproducible (VertX, Micronaut), then it should be easy to have a simple reproducer, right?

Look at the previous issue that was raised by @jameskleeh , he gives me enough elements to have a reproducer, and then I was able to fix it.

It seems, for what I believe, a bad usage of the Factory and Decoder, and in particular, the way to release all items.
I proposed to make a change to simplify it, but as it was a change of behavior (the way Memory based vs Disk based Factories are working), and most of all because I use a guard such as `buffer.refCnt() > 0` to release the underlying buffers, it was not accepted and I just then ensure the documentation says how it should be done.

     for (InterfaceHttpData httpData: decoder.getBodyHttpDatas()) {
         httpData.release();
         factory.removeHttpDataFromClean(request, httpData);
     }
     factory.cleanAllHttpData();
     decoder.destroy();

It might be releated to a wrong usage of the API, due to its complexity.

I believe that we should add (to be backward compatible) a special release method on both Factory and Decoder, to ensure that everything is clean up. But then, you will have to adapt the code, whatever the way.

So don't take me bad. Without help, I cannot help either. That's all I wrote.
Thanks for the clarification. A reproducer was provided with the issue report which currently reproduces the issue. You are asking for a pure Netty reproducer? That is very complex to setup given that Micronaut has a complete pipeline that is not easy to reproduce standalone. 
From other thread on other issue (I mixed wrongly the comments):

Javadoc for Factory:

    /**
     * Remove all InterfaceHttpData from virtual File storage from clean list for all requests
     */
    void cleanAllHttpData();

It means that only "file" based are released there, not "memory" based one.

Then on Decoder:

    /**
     * Destroy the {@link HttpPostMultipartRequestDecoder} and release all it resources. After this method
     * was called it is not possible to operate on it anymore.
     */
    @Override
    public void destroy() {
        // Release all data items, including those not yet pulled
        cleanFiles();

        destroyed = true;

        if (undecodedChunk != null && undecodedChunk.refCnt() > 0) {
            undecodedChunk.release();
            undecodedChunk = null;
        }
    }

Except that it calls `cleanFiles()` which calls ` factory.cleanRequestHttpData(request);` which Javadoc says:

    /**
     * Remove all InterfaceHttpData from virtual File storage from clean list for the request
     *
     * @param request associated request
     */
    void cleanRequestHttpData(HttpRequest request);

So yes, there is an issue, since only "Disk" based are free. And your codes are wrong.
This is what I point out.
For your intention, this issue was introduced 2 years ago (not by the last changes).

So, in order to propose something, what I can propose is to change as the following:


    /**
     * Destroy the {@link HttpPostMultipartRequestDecoder} and release all it resources. After this method
     * was called it is not possible to operate on it anymore.
     */
    @Override
    public void destroy() {
        // Release all data items, including those not yet pulled
        cleanFiles();
       
        // Explicitely remove all HttpData but only if refCnt is > 0
        for (InterfaceHttpData httpData: getBodyHttpDatas()) {
           if (httpData.refCnt > 0) {
               httpData.release();
          }
          factory.removeHttpDataFromClean(request, httpData);
        }
        destroyed = true;

        if (undecodedChunk != null && undecodedChunk.refCnt() > 0) {
            undecodedChunk.release();
            undecodedChunk = null;
        }
    }

You can do that however that also isn't related to this issue. This is about the speed of the upload which can be demonstrated before any data is released/destroyed
@jameskleeh  as @graemerocher refered to a memory issue, I believe that it was also the good place to put the same.

Now considering this: I really mismatched between your 2 issues. Too fast reading and answearing, my bad.

This exact issue is related to performance issue, which I already demonstrated that currently no issue is found without a reproducer.
Sorry about that...
I referred to both. To quote

> Netty behaviour has regressed such that we have memory leaks and performance problems 
Hey I am late as I just returned from vacation and have a huge backlog. That said I can try to have a look next week as well. As we know in which release the issue started it should be possible to figure out what is wrong 
@normanmaurer Thanks, since on performance issue, I don't know how to find out in such a huge code the reason.
On memory issue (other issue), I figured out (2 years ago change that I didn't do) and propose a fix right now.
@jameskleeh I checked your reproducer and got the memory leak as well with error:

```
java.lang.OutOfMemoryError: Direct buffer memory
        at java.base/java.nio.Bits.reserveMemory(Bits.java:175)
        at java.base/java.nio.DirectByteBuffer.<init>(DirectByteBuffer.java:118)
        at java.base/java.nio.ByteBuffer.allocateDirect(ByteBuffer.java:317)
        at io.netty.buffer.PoolArena$DirectArena.allocateDirect(PoolArena.java:645)
        at io.netty.buffer.PoolArena$DirectArena.newChunk(PoolArena.java:621)
        at io.netty.buffer.PoolArena.allocateNormal(PoolArena.java:204)
        at io.netty.buffer.PoolArena.tcacheAllocateNormal(PoolArena.java:188)
        at io.netty.buffer.PoolArena.allocate(PoolArena.java:138)
        at io.netty.buffer.PoolArena.reallocate(PoolArena.java:288)
        at io.netty.buffer.PooledByteBuf.capacity(PooledByteBuf.java:118)
        at io.netty.buffer.AbstractByteBuf.ensureWritable0(AbstractByteBuf.java:307)
        at io.netty.buffer.AbstractByteBuf.ensureWritable(AbstractByteBuf.java:282)
        at io.netty.buffer.AbstractByteBuf.writeBytes(AbstractByteBuf.java:1105)
        at io.netty.buffer.AbstractByteBuf.writeBytes(AbstractByteBuf.java:1098)
        at io.netty.buffer.AbstractByteBuf.writeBytes(AbstractByteBuf.java:1089)
        at io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.offer(HttpPostMultipartRequestDecoder.java:340)
        at io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.offer(HttpPostMultipartRequestDecoder.java:52)
        at io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.offer(HttpPostRequestDecoder.java:223)
        at io.micronaut.http.server.netty.FormDataHttpContentProcessor.onData(FormDataHttpContentProcessor.java:113)
        at io.micronaut.http.server.netty.AbstractHttpContentProcessor.doOnNext(AbstractHttpContentProcessor.java:78)
        at io.micronaut.http.server.netty.AbstractHttpContentProcessor.doOnNext(AbstractHttpContentProcessor.java:36)
        at io.micronaut.core.async.subscriber.CompletionAwareSubscriber.onNext(CompletionAwareSubscriber.java:52)
        at io.micronaut.http.netty.reactive.HandlerPublisher.publishMessage(HandlerPublisher.java:378)
        at io.micronaut.http.netty.reactive.HandlerPublisher.channelRead(HandlerPublisher.java:334)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
        at io.micronaut.http.netty.stream.HttpStreamsHandler.handleReadHttpContent(HttpStreamsHandler.java:292)
        at io.micronaut.http.netty.stream.HttpStreamsHandler.channelRead(HttpStreamsHandler.java:257)
        at io.micronaut.http.netty.stream.HttpStreamsServerHandler.channelRead(HttpStreamsServerHandler.java:121)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
        at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
        at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
        at io.netty.handler.codec.MessageToMessageCodec.channelRead(MessageToMessageCodec.java:111)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
        at io.netty.channel.ChannelInboundHandlerAdapter.channelRead(ChannelInboundHandlerAdapter.java:93)
        at io.netty.handler.codec.http.HttpServerKeepAliveHandler.channelRead(HttpServerKeepAliveHandler.java:64)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
        at io.netty.handler.flow.FlowControlHandler.dequeue(FlowControlHandler.java:200)
        at io.netty.handler.flow.FlowControlHandler.read(FlowControlHandler.java:139)
        at io.netty.channel.AbstractChannelHandlerContext.invokeRead(AbstractChannelHandlerContext.java:686)
        at io.netty.channel.AbstractChannelHandlerContext.read(AbstractChannelHandlerContext.java:671)
        at io.micronaut.http.netty.reactive.HandlerPublisher.requestDemand(HandlerPublisher.java:163)
        at io.micronaut.http.netty.stream.HttpStreamsHandler$2.requestDemand(HttpStreamsHandler.java:248)

```

However, reproducer says it uses` 4.1.59`, while in the ticket description you say "This started happening in 4.1.60 and is present in 4.1.63". Could you please clarify?

Also, I can't reproduce the memory leak with uncommented "4.1.63.Final". But I see visible performance degradation. Could you please clarify - with `4.1.63.Final` do you see a memory leak or performance issue or both?

Stacktrace reminds me of this issue - https://github.com/netty/netty/issues/10281 I fixed it by overriding the `HttpPostMultipartRequestDecoder`. We have a lot of upload tests with paranoid level and everything is ok with the overrided version (the use case is the same as in your example).
@doom369 The `upload_perf` branch of the repo uses `4.1.63`, so that is designed to show the performance issue. I was relatively certain I did reproduce the memory leak in `4.1.63`, however I just went through the exercise of waiting for x requests and I can not reproduce it either. I'll close the other issue and we can just leave this one for the performance issue. I appreciate your help

@jameskleeh @fredericBregier here are my findings so far.

Flame Graph for (netty 63 where performance issue is present):

[flame_63.zip](https://github.com/netty/netty/files/6369135/flame_63.zip)

Flame Graph for (netty 59 where no performance issue, but memory leak is present):

[flame_59.zip](https://github.com/netty/netty/files/6369137/flame_59.zip)

So the problem seems like in `io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.loadDataMultipartOptimized` method. @fredericBregier could you please take a look?

I wasn't able to reproduce it in our tests. (Probably because we have overridden `HttpPostMultipartRequestDecoder`).
But in the provided reproducer of @jameskleeh upload is ~100 times slower, than it should be.






@doom369 Thank you !
Strange, but as there is also another improvement in the current stream (#11145), could you recheck against the current 4.1 branch?

Strange since the difference between previous behaviour (59) and (63) is:
- 59 is usung getByte for each and every bytes when searching for delimiter, which cause a huge cost, and in particular using PARANOID detection mode, but even on SIMPLE or NONE detection mode
- 63 is using bytesBfore to search delimiter, which is really more efficient, whatever detection mode, even if PARANOID is still slower (independent of this codec, related to the way detection mode changed)
- current 4.1 (including patch) is using in addition a more efficient way to search for CRLF when no delimiter is found, by limiting the search interval from the end of the buffer (for instance if the buffer is 10K, it will only search in the last "delimiter.size" interval)

And I do use benchmark with async-profile and it shows the great improvement (comparing 59 to after), not the contrary.

I will try to redo an async-profile ton try to understand, but if you can in the same time redo it using last 4.1 version from github (including the last merged patch), that could be useful.
@fredericBregier I'd be happy to test with the snapshot, however it was a great pain to get that working before. Are there instructions somewhere on how to generate snapshot artifacts locally? I can run `mvn install` but that doesn't produce the os specific binaries for xpoll etc..
@jameskleeh I'm using directly IntelliJ to make my tests. But obviously, within Netty code (since I think we cannot shared multiple projects as micronaut and netty at once, such as in Eclispe).
I believe there is a Wiki page that presents how to build Netty.
@fredericBregier I'm unable to find that info
@jameskleeh Maybe the following ones ?
- https://netty.io/wiki/developer-guide.html
- https://netty.io/wiki/native-transports.html
@doom369 I did tests again V59 and current v64.

1. Running Benchmark: `HttpPostMultipartRequestDecoderBenchmark`

2. Running a slightly modified Junit within `HttpPostRequestDecoderTest`: `testBIgFileUploadDelimiterInMiddleChunkDecoderMemoryFactory` and also Mixed and Disk Factory

### Junit tests as performance tests

Changes in `HttpPostRequestDecoderTest` renamed into `HttpMultipartBenchamrkTemporaryTest`:


    @Test
    public void testBIgFileUploadDelimiterInMiddleChunkDecoderDiskFactory() throws IOException {
        // Factory using Disk mode
        HttpDataFactory factory = new DefaultHttpDataFactory(true);
        for (int i = 0; i < 100; i++) { // CHANGE: Add LOOP but only 100 since too long due to disk access
            commonTestBigFileDelimiterInMiddleChunk(factory, false);
        }
    }

    @Test
    public void testBIgFileUploadDelimiterInMiddleChunkDecoderMemoryFactory() throws IOException, InterruptedException {
        // Factory using Memory mode
        Thread.sleep(20000);
        HttpDataFactory factory = new DefaultHttpDataFactory(false);
        for (int i = 0; i < 6000; i++) { // CHANGE: Add LOOP
            commonTestBigFileDelimiterInMiddleChunk(factory, true);
        }
    }

    @Test
    public void testBIgFileUploadDelimiterInMiddleChunkDecoderMixedFactory() throws IOException {
        // Factory using Mixed mode, where file shall be on Disk
        HttpDataFactory factory = new DefaultHttpDataFactory(10000);
        for (int i = 0; i < 100; i++) { // CHANGE: Add LOOP but only 100 since too long due to disk access
            commonTestBigFileDelimiterInMiddleChunk(factory, false);
        }
    }

    private void commonTestBigFileDelimiterInMiddleChunk(HttpDataFactory factory, boolean inMemory)
            throws IOException {
        int nbChunks = 300; // CHANGE here from 30 to 300 to get 30 MB transfer
        int bytesPerChunk = 100000;
        int bytesLastChunk = 10000;

Times are the following:
- V59 : about 12 minutes for each (Disk or Mixed Factory with 100 about the same time than 6000 using Memory Factory) (except PARANOID mode where more than 2 hours and need to stop)
- V64 : about 2 minutes for each (whatever PARANOID or not)

I generate only for Memory based the Flame and standard async-profile outputs, both for V59 and V64, using or not PARANOID level. I had to stop after 2 hours the V59 in PARANOID mode since still not ending.

[TestV59vsV64.zip](https://github.com/netty/netty/files/6380056/TestV59vsV64.zip)


### On Benchmarks side

It uses lot of small files or a big one, depending on the tests, and use all kind of leak detection from NONE to PARANOID

- V59

      Benchmark                                                                           Mode  Cnt  Score    Error   Units
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel   thrpt    6  0,110 ±  0,115  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel   thrpt    6  0,678 ±  0,295  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel   thrpt    6  0,001 ±  0,001  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel     thrpt    6  0,442 ±  0,150  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel  thrpt    6  0,063 ±  0,021  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel  thrpt    6  0,460 ±  0,055  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel  thrpt    6  0,001 ±  0,001  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel    thrpt    6  0,504 ±  0,070  ops/ms

      Benchmark result is saved to /home/frederic/Waarp2/netty/microbench/target/reports/performance/HttpPostMultipartRequestDecoderBenchmark.json
      [835,175s][info   ][gc,heap,exit ] Heap
      [835,175s][info   ][gc,heap,exit ]  garbage-first heap   total 258048K, used 55694K [0x0000000707800000, 0x0000000800000000)
      [835,175s][info   ][gc,heap,exit ]   region size 1024K, 54 young (55296K), 2 survivors (2048K)
      [835,175s][info   ][gc,heap,exit ]  Metaspace       used 9371K, capacity 9763K, committed 10112K, reserved 1058816K
      [835,175s][info   ][gc,heap,exit ]   class space    used 1036K, capacity 1132K, committed 1152K, reserved 1048576K

- V64:

      Benchmark                                                                           Mode  Cnt  Score   Error   Units
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel   thrpt    6  5,318 ± 0,199  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel   thrpt    6  4,978 ± 1,401  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel   thrpt    6  0,620 ± 0,164  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel     thrpt    6  4,362 ± 0,355  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel  thrpt    6  1,900 ± 0,619  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel  thrpt    6  2,463 ± 0,176  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel  thrpt    6  0,146 ± 0,014  ops/ms
      HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel    thrpt    6  2,352 ± 0,139  ops/ms

      Benchmark result is saved to /home/frederic/Waarp2/netty/microbench/target/reports/performance/HttpPostMultipartRequestDecoderBenchmark.json
      [811,531s][info   ][gc,heap,exit ] Heap
      [811,531s][info   ][gc,heap,exit ]  garbage-first heap   total 258048K, used 56735K [0x0000000707800000, 0x0000000800000000)
      [811,531s][info   ][gc,heap,exit ]   region size 1024K, 55 young (56320K), 2 survivors (2048K)
      [811,531s][info   ][gc,heap,exit ]  Metaspace       used 9349K, capacity 9715K, committed 9856K, reserved 1058816K
      [811,531s][info   ][gc,heap,exit ]   class space    used 1035K, capacity 1132K, committed 1152K, reserved 1048576K

So roughly 10 times bette with V64.

So I cannot explain this performance issue.

@doom369 
As I canot reproduce within Netty, I can try to execute manually (thourgh IntellJ) the test from micronaut to see exactly whats is going on on your side.

Could you give me the setup and which test to run?
@fredericBregier @doom369 
I've given a quick look at the flamegraphs from @doom369 and, as the author of the optimized index of, I can say:

- it could be an issue with index of implementation, failing to find some specific byte and leaving the algo to go into some weird logic code path
- it could be an uncovered logical (corner) case with the find delimiter logic and/or caller, making it to call index of more and more (or on larger and larger chunks)

The former should be saved thanks to the coverage we already have on the test suite re index of.
The latter seems the most realistic candidate, although it's surprising there is no way to isolate this with vanilla Netty.
@franz1981 I agree, so the reason I would try to check manually the test from Micronaut under IntellJ to check the underlying corner that I can't fiigure out within vanilla Netty.

@doom369 So the reason I would like some help (guideline) to run this (or those) test(s) (which ones?) from Micronaut on my own to make debugging and trace.
It was the case for instance from another bug (fixed recently) from Micronaut too, but I was able to find out the logic in vanilla Netty.
My goal is to be able to reproduce within Micronaut, then within vanilla Netty.
@fabienrenaud I would be able to test only on Saturday. I'm not familiar with Micronaut as well so It will require some time to set up it with the snapshot version. I just run the provided reproducer from the initial desctioption, I was expecting to see some obvious thing to fix :). But looks like that not the case.
@doom369 @fredericBregier Once you have Netty in your local maven cache its simply a matter of changing the `build.gradle` file to point to that version.

Just change the string here https://github.com/jameskleeh/netty-memory-leak/blob/upload_perf/build.gradle#L28 then follow the steps in the README to run the app and upload a file:

https://github.com/jameskleeh/netty-memory-leak/blob/upload_perf/README.md

@doom369 @jameskleeh @franz1981 
I did run your code: with a file of 25 MB (zip file of 25577548 bytes)
- vs 59 : 0.727s at best each curl (the 5th through a memory Direct buffer exception)
- vs 64 : 0.115s each curl  in average with 1000 curl command (with a for)

So first the time is good with v64.

Now here are the flame and tree files.

[profile-micronaut-netty.zip](https://github.com/netty/netty/files/6388095/profile-micronaut-netty.zip)

What I see:
- V59 : 56% from loadDataMultipartStandard with self 8.45%

      [53] 56.41% 387 self: 8.45% 58 io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.loadDataMultipartStandard

- V64: 32% from loadDataMultipartOptimized with self 0.36% and findDelimiter 1.08%, so almost half

      [52] 32.43% 180 self: 0.36% 2 io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.loadDataMultipartOptimized
          [53] 25.23% 140 self: 1.08% 6     io/netty/handler/codec/http/multipart/HttpPostBodyUtil.findDelimiter

So I can't still explain what you have since even with your reproducer, I see no issue.
I surely missed something.
Just given a quick look at the flames from @fredericBregier and the sample count match my expectation (if I am reading it right) for cases where finding the delimiter is the bottleneck, see results on https://github.com/netty/netty/pull/10737#issuecomment-720481198 with high number of permutations and input size: the SWAR index of is at best twice as faster then reading single byte each time (for 32 long inputs, yet to see with more).
The samples on the flamegraphs are 381 samples loadDataMultiPartStandard vs 175 samples for loadDataMultiPartOptimized, meaning that the time spent on it is halved (ie assuming the same outcomes and the amount of work performed during the elapsed time is the same, it consumes half CPU time and/or is twice faster). Results related to the elapsed time shows 6X shorter elapsed time to perform the same work, so probably the gap is far more then the expected 2X because the original readByte used on V59 was getting a bound check + ref cnt check for each byte read.

I am a bit surprised how GC is stressed by 59 and how different is the I/O utilization of the 2 versions for both file read and networking. 

@fredericBregier It may be that the changes that were made after 63 was released fixed the issue. Can you verify you see the problem with 63?
@jameskleeh I will check when I have some time, but yes, I will ! ;-)
The result with V63 is very bad:
- v63 : 61.500s in average

The flame (with 5 transfers with the very same file):

[profile-micronaut-netty63-flame.zip](https://github.com/netty/netty/files/6392779/profile-micronaut-netty63-flame.zip)

It seems the change made in V64 are very critical (probably linked to both previous patch on your 15 days first ssue and your 2 last current issues).
@fredericBregier Then what is the chance we can get a 64 release in the near future?
@jameskleeh pretty good :) I think next week should be possible easily 
@fredericBregier do you know which change ? Also did the testing include your pending PR or not ?
@normanmaurer The tests are including the 3 PR:
- the current one: #11188 (Fix Memory release not correctly in Multipart Decoder)
- the 2 weeks ago one: #11145 (Fix behavior of HttpPostMultipartRequestDecoder for Memory based Factory)
- yours  #11207  (Destroy HttpPostMultipartRequestDecoder if contructor throws)

The changes are about:
- speed up in CRLF finding (and fix) within  `loadDataMultipartOptimized` (HttpPostMultipartRequestDecoder) and `findLastLineBreak` (HttpPostBodyUtil)
- freeing Memory each time possible (mainly was in constructor but you did included it in another one and also in `destroy` (HttpPostMultipartRequestDecoder)
@normanmaurer Do you know when 64 will go out? I'd like to do a new release of Micronaut with it asap
@jameskleeh next week

Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java
index af3b5022a23..5099ee60ac4 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java
@@ -31,6 +31,7 @@
 import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.NotEnoughDataDecoderException;
 import io.netty.util.CharsetUtil;
 import io.netty.util.internal.InternalThreadLocalMap;
+import io.netty.util.internal.PlatformDependent;
 import io.netty.util.internal.StringUtil;
 
 import java.io.IOException;
@@ -199,12 +200,17 @@ public HttpPostMultipartRequestDecoder(HttpDataFactory factory, HttpRequest requ
         }
         currentStatus = MultiPartStatus.HEADERDELIMITER;
 
-        if (request instanceof HttpContent) {
-            // Offer automatically if the given request is als type of HttpContent
-            // See #1089
-            offer((HttpContent) request);
-        } else {
-            parseBody();
+        try {
+            if (request instanceof HttpContent) {
+                // Offer automatically if the given request is als type of HttpContent
+                // See #1089
+                offer((HttpContent) request);
+            } else {
+                parseBody();
+            }
+        } catch (Throwable e) {
+            destroy();
+            PlatformDependent.throwException(e);
         }
     }
 


Test patch:
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java
index 696c2cb20a9..419e93beb1d 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java
@@ -15,11 +15,13 @@
  */
 package io.netty.handler.codec.http.multipart;
 
+import io.netty.buffer.Unpooled;
 import io.netty.handler.codec.http.DefaultFullHttpRequest;
 import io.netty.handler.codec.http.FullHttpRequest;
 import io.netty.handler.codec.http.HttpHeaderNames;
 import io.netty.handler.codec.http.HttpMethod;
 import io.netty.handler.codec.http.HttpVersion;
+import io.netty.util.CharsetUtil;
 import org.junit.Test;
 
 import static org.junit.Assert.assertTrue;
@@ -55,4 +57,28 @@ public void testDecodeFullHttpRequestWithInvalidCharset() {
             assertTrue(req.release());
         }
     }
+
+    @Test
+    public void testDecodeFullHttpRequestWithInvalidPayloadReleaseBuffer() {
+        String content = "\n--861fbeab-cd20-470c-9609-d40a0f704466\n" +
+                "Content-Disposition: form-data; name=\"image1\"; filename*=\"'some.jpeg\"\n" +
+                        "Content-Type: image/jpeg\n" +
+                        "Content-Length: 1\n" +
+                        "x\n" +
+                        "--861fbeab-cd20-470c-9609-d40a0f704466--\n";
+
+        FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload",
+                Unpooled.copiedBuffer(content, CharsetUtil.US_ASCII));
+        req.headers().set("content-type", "multipart/form-data; boundary=861fbeab-cd20-470c-9609-d40a0f704466");
+        req.headers().set("content-length", content.length());
+
+        try {
+            new HttpPostMultipartRequestDecoder(req);
+            fail("Was expecting an ErrorDataDecoderException");
+        } catch (HttpPostRequestDecoder.ErrorDataDecoderException expected) {
+            // expected
+        } finally {
+            assertTrue(req.release());
+        }
+    }
 }