Skip to content

Conversation

coszio
Copy link
Contributor

@coszio coszio commented Feb 13, 2025

This PR adds an iterator adapter to make hardware usage counting easier when we are measuring iterators.

This adapter plugs in easily to an iterator. When the iterator gets dropped, it will execute the callback to do something with the final number of iterations the iterator was used for.

For example:

index.value_to_points.get_iter(value).on_final_count(|nexts| {
    hw_counter
        .payload_index_io_read_counter()
        .incr_delta(nexts * size_of::<PointOffsetType>)
});

I am not sure about the name, though. Feel free to propose another one :)

Copy link
Contributor

@JojiiOfficial JojiiOfficial left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice addition. Thanks!

/// including the final one where it usually returns `None`.
///
/// Consider subtracting 1 if the final `None` is not needed.
fn on_final_count<F>(self, f: F) -> OnFinalCount<Self, F>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit verbose, but how about count_callback_on_drop? It might give a better hint on what it does without looking at the function.

I've no strong opinion on either.

cc @JojiiOfficial

Copy link
Contributor

@JojiiOfficial JojiiOfficial Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with both but agree this one being more self-explanatory

@JojiiOfficial JojiiOfficial requested a review from generall March 6, 2025 08:54
@JojiiOfficial JojiiOfficial merged commit d773328 into dev Mar 6, 2025
17 checks passed
@JojiiOfficial JojiiOfficial deleted the on_final_count-iter branch March 6, 2025 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants