Make --trace-gc-nvp less verbose. [v8/v8 : main]
Dominik Inführ (Gerrit)
Dominik Inführ added 1 comment![]( "Open in Gerrit")
if (v8_flags.trace_gc_nvp) [[unlikely]] {Since I added this and no one else is using this: We could also just drop this line?
Related details
- Erik Corry
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Erik Corry (Gerrit)
Erik Corry added 1 comment![]( "Open in Gerrit")
if (v8_flags.trace_gc_nvp) [[unlikely]] {Since I added this and no one else is using this: We could also just drop this line?
Unconditionally drop the PrintWithTimestamp, or unconditionally drop the 'if'?
I think the print might be a little useful to us in its less verbose form, but I'm open to dropping it completely.
Related details
- Dominik Inführ
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
Dominik Inführ voted and added 2 comments![]( "Open in Gerrit")
Votes added by Dominik Inführ
| Code-Review | +1 |
2 comments
if (v8_flags.trace_gc_nvp) [[unlikely]] {Erik CorrySince I added this and no one else is using this: We could also just drop this line?
Unconditionally drop the PrintWithTimestamp, or unconditionally drop the 'if'?
I think the print might be a little useful to us in its less verbose form, but I'm open to dropping it completely.
Sorry.. Yes, I meant to remove both the PrintWithTimestamp call and its surrounding if. I don't use this anymore and by now I think it's better to just report this from the caller, so e.g. UpdateAllocationLimits(). Only in the caller we have all the information/counter that the new limits are based on. I mostly added this print statement here to not miss any update.
Related details
- Erik Corry
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
Dominik Inführ voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dominik Inführ
| Code-Review | +1 |
1 comment
Erik CorrySince I added this and no one else is using this: We could also just drop this line?
Dominik InführUnconditionally drop the PrintWithTimestamp, or unconditionally drop the 'if'?
I think the print might be a little useful to us in its less verbose form, but I'm open to dropping it completely.
Sorry.. Yes, I meant to remove both the PrintWithTimestamp call and its surrounding if. I don't use this anymore and by now I think it's better to just report this from the caller, so e.g. UpdateAllocationLimits(). Only in the caller we have all the information/counter that the new limits are based on. I mostly added this print statement here to not miss any update.
Done
Related details
- Erik Corry
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Erik Corry (Gerrit)
Erik Corry voted Commit-Queue+2![]( "Open in Gerrit")
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Unreviewed changes
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Make --trace-gc-nvp less verbose.
Change-Id: I9e2623ca482af0ebca335c27fb1bbb2d734052dc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7368313
Commit-Queue: Erik Corry <erik...@chromium.org>
Reviewed-by: Dominik Inführ <dinf...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#104565}
- M src/heap/heap.cc
Code-Review: +1 by Dominik Inführ
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |