search

capnproto / capnp-ocaml

OCaml code generator plugin for the Cap'n Proto serialization framework
Other
98 stars 20 forks source link

Implement pooling of message buffers #69

Closed Cjen1 closed 4 years ago

Cjen1 commented 4 years ago

In the current api there is the capability to pool message buffers: 

Message.Make(...) = sig
val alloc : ...
val release: ...

This PR implements this functionality by use of the buffer-pool-min package. The use of buffer-pool-min rather than buffer-pool allows for no breaking API changes, however there will be memory leak style behavior with randomly sized messages.

The main benefit of this is for workloads which mainly involve similarly sized messages. This would allow them to minimise the GC work done (currently ~50% of the CPU time is spent on GC...).

The implementation of a pooled store, however some of the tests are not passing and I am not sure why yet...

Cjen1 commented 4 years ago

Currently blocked on buffer-pool getting merged into opam_repo

talex5 commented 4 years ago

currently ~50% of the CPU time is spent on GC...

Is that still the case? https://github.com/mirage/capnp-rpc/pull/200 made the benchmark 1.7x faster, so I would assume the time spent on GC went down (but maybe something else is going on).

Cjen1 commented 4 years ago

So for my use case in OcamlPaxos the message sizes are relatively large. So the GC cost is still large however the buffer pool scheme should help. That being said in the benchmark there does not appear to be any benefit to buffer pooling currnetly (sorry my 50% number was old).

Although I'm checking currently to see whether the buffers are getting released in capnp-rcp. (its currently looking like they aren't calling Message.release anywhere [from the call graph]...)

talex5 commented 4 years ago

Oh. How come the paxos messages are so big? I'm a bit nervous about the pool thing because it's really easy to mess it up and reuse a buffer that's still being used somewhere. That can be be very hard to debug!

Cjen1 commented 4 years ago

So the major cause of the large messages is that we're including user data within the client requests, which can be of arbitrary length.

But since there is a relatively stable distribution of data lengths buffer pooling would help to avoid lots of the overhead in allocations.

I completely understand re it being error prone (see c++) I'm just hoping that there would be a good insertion point (for example the release_param_caps call) for the release calls. But it does seem non-obvious where to insert it.