Improve memory usage via reuse InputStreamBufferInput

[koo-taejin]

I am testing to use MsgPack instead of Jackson in Spring. It was confirmed that MsgPack consumes more memory than Jackson.

I dug into that issue, it was confirmed that a lot of memory is created when creating InputStreamBufferInput. So I have make code very similar to the previous reuse form.

After the change, I had confirmed that the memory usage was greatly reduced, and the slope of the heap memory was also changed to be very stable.

I didn't add a test because I don't know whether you merge the code or not, but if you are going to accept it, I'll add a simple test code.

• as-is

• to-be

Thanks :)

[komamitsu]

@koo-taejin Great suggestion. Thanks for looking into the issue. And sorry for the late reply. I looked at https://github.com/koo-taejin/msgpack-java/commit/7e59e496fc1c233d06f0172ceaceefd100469017 and the basic idea seems good. But I think there is room to improve it as follows:

1. This MessageBufferInput (InputStreamBufferInput) cache can be treated as a part of existing reuseResourceInParser feature. So, we can avoid introducing the new feature flag reuseInputStreamBufferInput.
2. Maintaining the different cache lifetimes of MessageUnpacker and MessageBufferInput would increase the complexity. We should keep using the same ThreadLocal to hold the new resource MessageBufferInput by change the value type from Tuple<> to Triple<Object, MessageUnpacker, MessageBufferInput>. I think this requires to modify some existing code.

If you want to move forward on this improvement and create a PR, I hope the above idea sounds reasonable to you. If you're busy or not interested in creating a PR, I think I can take over this performance improvement as I have some time now.

[koo-taejin]

@komamitsu I will try to change the code according to your suggestion. :) Thanks for letting me know for this project implementation more.

[komamitsu]

Great to hear that!

I think we need to delay to create a MessageBufferInput instance until https://github.com/msgpack/msgpack-java/blob/edd094282368d26fff7808ca792faa0ce488be61/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackParser.java#L169. It means the private constructor needs to receive both of byte[] and InputStream (or either of them in a smart way) as argument.

[koo-taejin]

@komamitsu

Please check this PR is what you want. If it is right, then I am going to add test code. And If you want to change my code, then you can change it viamaintainer option.

Thanks :)

[komamitsu]

@koo-taejin Thanks. Looked over the branch (not PR, right?) Unfortunately, the change is too much more than I expected. Especially core component shouldn't be changed as much as possible since it's been carefully tuning considering JIT (e.g. Type Profile.) If it's okay, can I take over your branch and create a PR?

[koo-taejin]

@komamitsu Absolutely yes, that is my honor. :)

[komamitsu]

@koo-taejin I tried to reproduce the memory consumption difference with your change based on https://github.com/msgpack/msgpack-java/blob/develop/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/benchmark/MessagePackDataformatPojoBenchmarkTest.java, but I didn't see any significant difference (I used jcmd PID GC.class_histogram.) Could you give me the test code to reproduce the memory consumption?

[koo-taejin]

@komamitsu

I had tested to apply msgpack as a message protocol to spring mvc via following kotlin code.

   val msgPackCoverter = object : AbstractJackson2HttpMessageConverter(
            ObjectMapper(MessagePackFactory()),
            MediaType("application", "x-msgpack")
        ) {}

thanks :)

[komamitsu]

@koo-taejin Thanks for the information. But even based on the code you shared above, I don't think we can add tests code that is needed to see if the fix actually works. Do you think you can give me minimum code that reproduces how much your change improves the memory consumption?