facebookexperimental / object-introspection

Object Introspection (OI) enables on-demand, hierarchical profiling of objects in arbitrary C/C++ programs with no recompilation.
Apache License 2.0
163 stars 14 forks source link

Cached data for `oi::SizedResult` is stored in the short lived iterator #479

Open JakeHillion opened 8 months ago

JakeHillion commented 8 months ago

SizedResult uses a two pass algorithm to provide total size for every element when iterating an oi::IntrospectionResult. In the first pass it fills up a std::vector<SizeHelper> to make the calculation of size trivial. In the second pass it returns the oi::result::Element from the oi::IntrospectionResult::const_iterator with the calculated size to return an oi::result::SizedResult.

The current approach stores the std::vector<SizeHelper> in the const_iterator and fills it when the const_iterator is constructed. Filling the vector when the iterator is constructed makes sense as it delays the work until it is used. The downside of this approach is if you create two const_iterators for the oi::SizedResult (a natural way to use it) you do this work twice.

For example

auto res = oi::result::SizedResult{oi::introspect(...)};
auto totalSize = res.begin()->size;  // fills the entire std::vector<SizeHelper> and destroys it with the iterator
for (const auto& el : res) { ... } // fills the entire std::vector<SizeHelper> again and destroys it at the end of the loop

It would be relatively simple to store this std::vector<SizeHelper> in the SizedResult and store a reference to it in the SizedResult::const_iterator. When creating a const_iterator you would check if the cache vector is empty, and if so attempt to fill it. Then the SizedResult::const_iterator could hold a std::span<const SizeHelper> to this vector instead of owning it. Future iterators would not need to redo this work. As the underlying result is unchanged and traversing it is deterministic there's no problem with this caching.