Skip to content

Added performance counters to zippers - #14

Open
imlvts wants to merge 3 commits into
masterfrom
zipper_counters
Open

Added performance counters to zippers#14
imlvts wants to merge 3 commits into
masterfrom
zipper_counters

Conversation

@imlvts

Copy link
Copy Markdown
Collaborator

This introduces timed_span macros used in all the zipper methods.
The macro uses a global array for each span, incrementing times the span was entered, and platform-specific TSC delta spent in the span.
The counters may be either read from a static array, or printed using pathmap::timed_span::print_counters();.

@imlvts
imlvts changed the base branch from master to cleanup_to_releaseJune 3, 2025 18:43
@luketpeterson

Copy link
Copy Markdown
Collaborator

Hey. Sorry I didn't see this PR amid all the other churn.

High-level questions:

  • Is this feature intended for internal use (optimizing pathmap) or external use (helping users optimize code that uses pathmap)?
  • Is the cost negligible to have it always on? Or should be be behind a feature flag when it's not needed?
  • Is it going to cause a problem that some zipper implementations provide their own implementations of some methods, and therefore won't call the default impls with the timed_span! macro invocations in them?

@Adam-Vandervorst

Copy link
Copy Markdown
Owner

Can we pull this at least into a non master branch @luketpeterson ?

@Adam-Vandervorst

Copy link
Copy Markdown
Owner

Bump @luketpeterson

@luketpeterson
luketpeterson changed the base branch from cleanup_to_release to masterSeptember 6, 2025 00:27
@luketpeterson

Copy link
Copy Markdown
Collaborator

I have no problem main-lining this, but if it's targeted towards internal optimization then it probably belongs behind a feature gate.

@imlvts, What are your opinions on these questions? #14 (comment)

Sign up for freeto 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

@imlvts@luketpeterson@Adam-Vandervorst