Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
JakeHillion opened this issue Feb 6, 2024 · 0 comments
Open
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@JakeHillion
Copy link
Contributor

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.

@JakeHillion JakeHillion added enhancement New feature or request good first issue Good for newcomers labels Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant