search

boostorg / accumulators

An awesome library from Boost
http://boost.org/libs/accumulators
22 stars 54 forks source link

Allow accumulator persistency #18

Closed yuvalif closed 5 years ago

yuvalif commented 5 years ago

This PR is meant to address this request (also tracked as issue #13). The great value from accumulators is the fact there is no need to store large amounts of raw data, in order to get the statistics. Allowing serialization of internal state, so that the process accumulating the stats, could stop and continue from where it left off, would allow for more scenarios to use accumulators without the need to store the raw data.

  1. there should be no perf impact on regular operation of the accumulators
  2. rolling stats are not serializeable yet (more work is needed for serializing the circular buffer)
  3. In some cases, the ctor parameters are stored inside the accumulator, so, when deserialized they are overwritten. Was not sure if this is what would be the expected behavior?
  4. made some loops longer to fix flaky tests (unrelated to this PR), addressing issue #12
ericniebler commented 5 years ago

VERY cool. I don't have time to fully review right now. I'm a little concerned about "rolling stats are not serializeable yet". Should I give you more time to finish this up?

yuvalif commented 5 years ago

@ericniebler will gladly add that. However, to prevent dependencies with a PR to boots/serialization, I'll just add the circular buffer serialization support here. Will try to push that as a PR to the serialization repo as a separate effort (and maybe converge later).

yuvalif commented 5 years ago

added rolling stats persistency and tests. boost::circular_buffer serialization was added locally to prevent dependency issues. Started a PR (https://github.com/boostorg/circular_buffer/pull/25) on the serialization repo for the circular buffer so that the code could end up eventually in the right place. will remove from here, once the other PR is merged.

redlars commented 5 years ago

This is a very interesting feature.

What is the status of this work and when is it planned to be included in a Boost release?

yuvalif commented 5 years ago

@redlars the work on this feature is complete, and waiting for review. The only point for discussion is for the circular buffer serialization. In this PR is is done within the specific context of accumulators (the rolling window accumulators use it). In parallel I started another PR, for more general circular buffer serialization. However, It does not seem like there is an agreement on the necessity of that feature in the general case for circular buffer. So, I would prefer to accept this PR as-is, and modify the code, in the future, if circular buffer serialization will be added to the circular buffer code.

ericniebler commented 5 years ago

I've merged this. Apologies for the lag. Accumulators currently lacks an active maintainer.