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

Change ZAM Profiling to use per-thread CPU timing #3659

Open
timwoj opened this issue Mar 19, 2024 · 0 comments
Open

Change ZAM Profiling to use per-thread CPU timing #3659

timwoj opened this issue Mar 19, 2024 · 0 comments

Comments

@timwoj
Copy link
Contributor

timwoj commented Mar 19, 2024

I've brought this up quite a while ago and still holding onto it: This should use CPU time of the main/current thread rather than CPU time of the full process (as curr_CPU_time() currently seems to do). Zeek is multi-threaded, so this includes CPU time consumed by logger or other threads running independently and in parallel to script execution. It seems when pinning a zeek -r invocation to a single CPU this could skew and jitter things around.

Last time this came up with script profiling you mentioned "logging/caf" activity may be related to script activity and it would make sense to include in the profile (which I'd still challenge :-) ). However, this is now about individual statements and there's logic to to correct for measurement overhead. Seems to me excluding noise/cpu time used by parallel running threads would be more than reasonable for more consistent measurements first.

Originally posted by @awelzel in #3644 (comment)

The thread linked above has a bunch more context/ideas in it.

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

No branches or pull requests

1 participant