add DMU prefetch efficiency ratio

[shodanshok]

arc_summary currently tracks two prefetch performance info:

• data prefetch efficiency, which measures how many prefetches were satisfied by ARC;
• DMU hits/misses, which measure how many times the prefetch fired or not (when it was engaged or not).

This patch adds a key missing performance metric: how many of the prefetch-triggered I/O operations where effectively useful. It does that by computing the ratio between the values arcstats/demand_hit_predictive_prefetch and zfetchstats/io_issued

Signed-off-by: Gionatan Danti [email protected]

Motivation and Context

Description

How Has This Been Tested?

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[amotin]

It generally makes sense to me. I just think that terminology is confusing. May be we could rename "Hit ratio"/"Miss ratio" into "Stream hit ratio"/"Stream miss ratio", and the new metrics I'd call "Read-ahead efficiency", or inverted it to "False prediction ratio".

Or even better, since last several days I am actually working on refactoring arc_access() and around it, I could collect more data to report "Number of prescient prefetches", "False prescient prefetch ratio", "Late prescient prefetch ratio", "Number of speculative prefetches", "False speculative prefetch ratio" and "Late speculative prefetch ratio". I think it would be more informative.

[amotin]

Done: #14123 .

[shodanshok]

First, thanks for reviewing this small patch.

It generally makes sense to me. I just think that terminology is confusing. May be we could rename "Hit ratio"/"Miss ratio" into "Stream hit ratio"/"Stream miss ratio", and the new metrics I'd call "Read-ahead efficiency", or inverted it to "False prediction ratio".

I like "Stream hit/miss ratio", while I would avoid using "read-ahead" in place of "prefetch" because it can be confusing to introduce a new term for what ZFS itself calls prefetch. That said, I am not sure if changing these names is useful at all as we can introduce issues for those grepping arc_summary output.

Or even better, since last several days I am actually working on refactoring arc_access() and around it, I could collect more data to report "Number of prescient prefetches", "False prescient prefetch ratio", "Late prescient prefetch ratio", "Number of speculative prefetches", "False speculative prefetch ratio" and "Late speculative prefetch ratio". I think it would be more informative.

I am all for more info, but I would avoid showing all these by default. Maybe they can be presented when using raw output with arc_summary -r.

Just my 2c.

[amotin]

I am all for more info, but I would avoid showing all these by default.

I think both False and Late numbers are useful, since one means too much prefetch and another means not enough.

[shodanshok]

I am all for more info, but I would avoid showing all these by default.

I think both False and Late numbers are useful, since one means too much prefetch and another means not enough.

Yeah, I can see the usefulness of these two values.

Maybe this simpler patch, which does not depend on any ARC refactor, can be accepted as-is and backported to current 2.1.x releases, with your more comprehensive approach used for future releases where ARC refactor is merged.

[amotin]

Maybe this simpler patch, which does not depend on any ARC refactor, can be accepted as-is and backported to current 2.1.x releases, with your more comprehensive approach used for future releases where ARC refactor is merged.

I am not exactly happy with comparing counters from two different subsystems. While now they may be pretty close, my PR unleashes them a bit, that is why I've added other more coherent ones.

[shodanshok]

Sorry, I involuntarily deleted the changed files by syncing my branch with openzfs/master. If the patch is of any interest, I can open a new pull requests. Otherwise, let this be closed.

Thanks.

[amotin]

@shodanshok Please look on the above ^^^.

[shodanshok]

@amotin The patch looks good, I'll try your branch on a test VM. Thank you so much for addressing this issue.