Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2881943002:
Make window message processing tally in the task profiler. (Closed)
|
Created:
3 years, 7 months ago by Sigurður Ásgeirsson Modified:
3 years, 7 months ago CC:
chromium-reviews, grt+watch_chromium.org, danakj+watch_chromium.org, vmpstr+watch_chromium.org, wfh+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionMake window message processing tally in the task profiler.
As-is the processing and memory allocations taking place during window
message processing is invisible to the task profiler.
BUG=644385
Review-Url: https://codereview.chromium.org/2881943002
Cr-Commit-Position: refs/heads/master@{#472142}
Committed: https://chromium.googlesource.com/chromium/src/+/21a0c8f770ea811a3830d5f8c6447306a686a63b
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address Greg's comment. #Messages
Total messages: 24 (16 generated)
The CQ bit was checked by siggi@chromium.org to run a CQ dry run
siggi@chromium.org changed reviewers: + fdoray@chromium.org, gab@chromium.org
Francois, this looks to be a nice bottleneck for sent and posted messages, as we discussed earlier today. Gab for OWNERs, pretty please.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
fdoray@chromium.org changed reviewers: + grt@chromium.org
lgtm +grt/ for base/win/
Description was changed from ========== Make window message processing tally in the task profiler. As-is the processing and memory allocations taking place during window message processing is invisible to the task profiler. BUG=644385 ========== to ========== Make window message processing tally in the task profiler. As-is the processing and memory allocations taking place during window message processing is invisible to the task profiler. BUG=644385 ==========
gab@chromium.org changed reviewers: - gab@chromium.org
On 2017/05/15 17:16:43, fdoray wrote: > lgtm > > +grt/ for base/win/ grt is best for base/win/, removing self :)
https://codereview.chromium.org/2881943002/diff/1/base/win/wrapped_window_proc.h File base/win/wrapped_window_proc.h (right): https://codereview.chromium.org/2881943002/diff/1/base/win/wrapped_window_pro... base/win/wrapped_window_proc.h:78: tracked_objects::ScopedProfile scoped_profile( are you doing this in its own function template because stack unwinding won't run dtors of stack objects within the scope of the __try{} in case of an exception? if so, please document. if not, what is the benefit to this over putting scoped_profile within the __try{} block? i ask because with the two templates defined like this, i (the reader) wonder why i would use WrappedWindowProc instead of ProfiledWrappedWindowProc. the latter is the first thing i'll read, and sounds like a reasonable thing to use ("sure, i want profiling!").
The CQ bit was checked by siggi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks, PTAL? https://codereview.chromium.org/2881943002/diff/1/base/win/wrapped_window_proc.h File base/win/wrapped_window_proc.h (right): https://codereview.chromium.org/2881943002/diff/1/base/win/wrapped_window_pro... base/win/wrapped_window_proc.h:78: tracked_objects::ScopedProfile scoped_profile( On 2017/05/16 07:27:35, grt (UTC plus 2) wrote: > are you doing this in its own function template because stack unwinding won't > run dtors of stack objects within the scope of the __try{} in case of an > exception? if so, please document. if not, what is the benefit to this over > putting scoped_profile within the __try{} block? > > i ask because with the two templates defined like this, i (the reader) wonder > why i would use WrappedWindowProc instead of ProfiledWrappedWindowProc. the > latter is the first thing i'll read, and sounds like a reasonable thing to use > ("sure, i want profiling!"). This was pure habit - seems the ScopedProfile works just fine inside the __try block. The ScopedProfile won't get destructed on SEH, and this will mess up the cumulatives for this thread, but it don't matter none, cause there's much bigger problems, and the process isn't long for the world.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by siggi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from fdoray@chromium.org Link to the patchset: https://codereview.chromium.org/2881943002/#ps20001 (title: "Address Greg's comment.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494954224318040,
"parent_rev": "d6a0a2e456e9f9e938d72c80a681ea701bdec8ec", "commit_rev":
"21a0c8f770ea811a3830d5f8c6447306a686a63b"}
Message was sent while issue was closed.
Description was changed from ========== Make window message processing tally in the task profiler. As-is the processing and memory allocations taking place during window message processing is invisible to the task profiler. BUG=644385 ========== to ========== Make window message processing tally in the task profiler. As-is the processing and memory allocations taking place during window message processing is invisible to the task profiler. BUG=644385 Review-Url: https://codereview.chromium.org/2881943002 Cr-Commit-Position: refs/heads/master@{#472142} Committed: https://chromium.googlesource.com/chromium/src/+/21a0c8f770ea811a3830d5f8c644... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/21a0c8f770ea811a3830d5f8c644... |
