Make Shell::NotifyLowMemoryWarning trace #19283
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
dnfield
force-pushed
the
trace_notify_low_mem
branch
from
69bd89a to
838a89f
Compare
chinmaygarde
left a comment
chinmaygarde
left a comment
There was a problem hiding this comment.
Please use the TraceFlow RAII wrapper to trace this. When that is used, you won't need an extra ivar, and we will be able to trace overlapping flows (i.e, if the application calls a low memory notification before the previous one has fully propagated).
|
Oh, wait, you didn't want a Flow, well |
|
Done - using TraceNonce now. Should this be a flow event? I'm not always clear on how those are helpful |
chinmaygarde
left a comment
chinmaygarde
left a comment
There was a problem hiding this comment.
Should this be a flow event? I'm not always clear on how those are helpful
Personally, I find flows more useful when:
- There are tons of these traces in a short period of time.
- These traces are overlapping.
- I need to know specific slice the flows pass through.
It seems like none of these conditions hold here so your intuition about using async events here is spot on.
dnfield
added
the
waiting for tree to go green
6 participants
Add tracing to
Shell::NotifyLowMemoryWarning, since it can be expensive and we want to know if/when it's getting called.Not sure how to add a test for this - I don't think we typically test these.