Optimization of msgpack::zone size on the stack and deferred memory allocation

[Arenoros]

Reduced zone size from 54 to 24 bytes for x64 and added delayed allocation of the first chunk as needed.
In case when there are a lot of non-large objects that require serialization in msgpack::objects it is very expensive to allocate memory just to create a zone to use the adapter

template<>
struct msgpack::adaptor::object_with_zone<T> {
    void operator()(msgpack::object::with_zone& o, T v) const {
        o << v;
    }
};

[redboltz]

Thank you for sending the PR. I will review it. But I need a time, please wail a couple of weeks.

[codecov-commenter]

Codecov Report

Merging #1089 (add68ff) into cpp_master (37fcaa1) will not change coverage.
The diff coverage is 85.71%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##           cpp_master    #1089   +/-   ##
===========================================
  Coverage       85.75%   85.75%           
===========================================
  Files              80       80           
  Lines            5091     5091           
===========================================
  Hits             4366     4366           
  Misses            725      725           

[redboltz]

Let me clarify your issue.

I think that you can use msgpack::object without allocating zone.
Adaptor has object and object_with_zone. The formar is for the types that do not require zone.
However, in some of generic algorithm in your application, require or not requrie zone is unpredictable in the algorithm. So you always need to prepare zone. In this case, the class zone allocates memory from the heap in the constructor. You want to avoid that. The issue is heap allocation. Am I understanding correctly?
sizeof(zone) or sizeof(something is also the issue?

Which size is reduced to 24 bytes from 54 bytes?

[redboltz]

I guess that your issue is the following allocation. (original code)

msgpack-c/include/msgpack/v1/detail/cpp11_zone.hpp

Lines 111 to 114 in 37fcaa1

	struct chunk_list {
	chunk_list(size_t chunk_size)
	{
	chunk* c = static_cast<chunk*>(::malloc(sizeof(chunk) + chunk_size));

Is that right ?

[Arenoros]

Yes, the sizeof(zone) has changed.
Partially, I have code that links C++ objects to lua and netcore through serialization/deserialization of individual fields in msgpack. The API that reads and writes from/to lua doesn't know anything about the C++ types to be deserialized into, and obviously lua objects aren't known at compile time either. Because of this I always need object_with_zone because some fields can be both simple types and complex structures. Also storing a zone next to an object helps to save on unnecessary memory allocations for small objects. But to allocate it always, even where the fields of the object are all simple types, it is too expensive because there can be thousands of such objects.
I hope I was able to explain it clearly enough.

Due to the fact that msgpack header only library I have no problem with not putting this code into the general library for whatever reason, but I thought it might be useful to others as well.

[redboltz]

Thank you for the comment.

	HEAD	This PR	Note
zone	56	24	from 8+24+24=56 to 8+8+8=24
chunk_list	24	24
finalizer_array	24	8	removed two pointers

I understand the updating points.

Next step, I will do some performance tests.
Please wait a moment.

[redboltz]

No siginificant performance difference has been observed.
I think that your fix increases one memory allocation in the typical case but it seems that it can be igored.

Let's move on the next step.
I will request you to trivial fix (e.g. coding style).
Please wait a moment.

[redboltz]

Sorry for my late review. I had a cold for a long time.

I added review comments:

In addition, please apply the same fix to
https://github.com/msgpack/msgpack-c/blob/cpp_master/erb/v1/cpp03_zone.hpp.erb
and then, execute https://github.com/msgpack/msgpack-c/blob/cpp_master/preprocess to generate the C++03 codes form the erb template.
(ruby is required)
Finally, add a commit including erb and updated heders.

[Arenoros]

I also translated the process script completely into ruby, because it is not very convenient to run the generation under windows

[redboltz]

Thank you for updating.
It seems that your branch is created from a little old cpp_master. Could you rebase it from up to date one ? And then, push --force please.

[redboltz]

All windows builds are failed on github actions. I guess that the failure is caused by environmental issue.
That means your fix is not the reason. However, we need to fix it before the PR merging for the future.

I'm not sure the reason but dart support removal could be relased.

I also create #1095 to investigate the issue. It is just for testing. I will close #1095, when the issue would be solved. Unfortunately I don't have much time to investigate it.

Perhaps, the issue would be solved by the environment update. In this case, we need to just wait for updating.

If you find a solution, please add a commit.

[Arenoros]

Yes, I've seen the problem of plugging in dependencies, I think there was already a similar zlib issue recently. Unfortunately I have no experience with ci github and appveyor in particular, so I can't help much here.

[redboltz]

Yes, I've seen the problem of plugging in dependencies, I think there was already a similar zlib issue recently. Unfortunately I have no experience with ci github and appveyor in particular, so I can't help much here.

Thank you for the comment. I will investigate the issue more when I have a time.

NOTE: zlib issue has been solved by #1090 and #1091 . Fortunately they were simple version issue.

[redboltz]

I solved the issue.
The issue is caused by boost.system dependency. It had existed as the library but now it is a header-only. cmake or boost update on the windows CI causes the issue. So I removed the requirement (dependency).
This PR has been moved into #1096