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

Event recording & summarize need cleanup with respect to self-time vs incr-load-time vs blocked-time vs total-time. #75

Closed
michaelwoerister opened this issue Nov 6, 2019 · 2 comments · Fixed by #104

Comments

@michaelwoerister
Copy link
Member

michaelwoerister commented Nov 6, 2019

At the moment, we have not specified how self-time and other kinds of measurements interact with each other:

  • Does the incremental cache loading time contribute to a queries self-time?
  • In the incremental cache loading case, is the loading event nested inside a regular query-provider event?
  • Should we distinguish between actual cache loading, metadata loading, and provider re-executions caused by missing cache entries? If so, how?
  • Do query-blocked time contribute to self-time? How about total-time?
  • Is a query-blocked event always followed by a cache-hit event (the latter of which would then be responsible for incrementing the invocation count) What if cache-hit events are filtered out? Is it OK to have wrong invocation counts?
@michaelwoerister
Copy link
Member Author

My suggested answers to these questions are:

Does the incremental cache loading time contribute to a queries self-time?

Yes. Self-time is what one sorts for when trying to find out what is expensive. It would be unhelpful if something got sorted to the back even though 50% of total compile time is spent loading things for it from the cache.

In the incremental cache loading case, is the loading event nested inside a regular query-provider event?

Let's go with no. That causes fewer events to be generated. Also, in order to make things really uniform, in-memory cache hit events would have to wrapped with a query-provider event too, which would be lots of overhead for not much gain.

Should we distinguish between actual cache loading, metadata loading, and provider re-executions caused by missing cache entries? If so, how?

Down the line, yes. It's a useful distinction to make. These should all be separate query kinds (incr. cache loading already is). However, I think this is something to be implemented after we have query keys, so we don't make too many changes at once.

Do query-blocked time contribute to self-time? How about total-time?

Blocking should add to self-time for the same reason as incr. cache loading should add to self-time (see above). I don't remember what I meant by "How about total-time?", I think the question is not relevant anymore since we switched to @andjo403's approach of computing total time as thread_max_time - thread_min_time.

Is a query-blocked event always followed by a cache-hit event (the latter of which would then be responsible for incrementing the invocation count) What if cache-hit events are filtered out? Is it OK to have wrong invocation counts?

I haven't thought about this one yet.

@michaelwoerister
Copy link
Member Author

Is a query-blocked event always followed by a cache-hit event (the latter of which would then be responsible for incrementing the invocation count) What if cache-hit events are filtered out? Is it OK to have wrong invocation counts?

It turns out the natural way of implementing this is that a cache-hit event is indeed generated after each query-blocked event. So this does not behave differently than the regular case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant