Last Comment Bug 668355 - Measure plugin enumeration with telemetry
: Measure plugin enumeration with telemetry
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: unspecified
: x86 Linux
: -- normal (vote)
: mozilla7
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks: 614423
  Show dependency treegraph
 
Reported: 2011-06-29 14:31 PDT by (dormant account)
Modified: 2011-07-02 03:05 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
raii helper (843 bytes, patch)
2011-06-29 14:33 PDT, (dormant account)
mh+mozilla: review+
Details | Diff | Splinter Review
telemetry for addon scanning (6.18 KB, patch)
2011-06-29 14:43 PDT, (dormant account)
no flags Details | Diff | Splinter Review
raii helper (826 bytes, patch)
2011-06-30 15:50 PDT, (dormant account)
taras.mozilla: review+
Details | Diff | Splinter Review
telemetry for addon scanning (2.16 KB, patch)
2011-06-30 15:51 PDT, (dormant account)
justin.lebar+bug: review+
Details | Diff | Splinter Review

Description (dormant account) 2011-06-29 14:31:02 PDT

    
Comment 1 (dormant account) 2011-06-29 14:33:44 PDT
Created attachment 542953 [details] [diff] [review]
raii helper
Comment 2 (dormant account) 2011-06-29 14:43:34 PDT
Created attachment 542958 [details] [diff] [review]
telemetry for addon scanning

This measures time spent on enumeration.
As far as I can tell .javaEnabled is the leading trigger of plugin enumeration. This should confirm it.
Comment 3 Mike Hommey [:glandium] 2011-06-29 22:16:06 PDT
Comment on attachment 542953 [details] [diff] [review]
raii helper

Review of attachment 542953 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/telemetry/Telemetry.h
@@ +75,5 @@
> +
> +private:
> +  const ID id;
> +  const TimeStamp start;
> +};

I think this would also work with something like:
template <int id>
class Timer {
public:
   Timer(): start(TimeStamp::Now()) {}
   ~Timer() { Accumulate(ID(id), (TimeStamp::Now() - start).ToMilliseconds()); }

private:
   const TimeStamp start;
};

In practice, with compiler optimizations, I'm not sure it would make any difference, though. (except in debug builds where we build without optimization)
Comment 4 (dormant account) 2011-06-30 15:50:02 PDT
Created attachment 543285 [details] [diff] [review]
raii helper

Template suggestion makes using this brilliant. Thanks.
Comment 5 (dormant account) 2011-06-30 15:51:03 PDT
Created attachment 543286 [details] [diff] [review]
telemetry for addon scanning

Updated with template change
Comment 6 (dormant account) 2011-06-30 15:52:52 PDT
Josh, I would appreciate a quick review on this. Would be good to get some wider coverage on plugin overhead in released versions in firefox(since we are merging soon)
Comment 7 Justin Lebar (not reading bugmail) 2011-07-01 11:46:43 PDT
Comment on attachment 543286 [details] [diff] [review]
telemetry for addon scanning

This looks fine to me, although I'd prefer Telemetry::Timer were called AutoTimer to emphasize the fact that it's RAII.
Comment 9 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-07-01 13:58:14 PDT
Would be nicer if this were AutoTimer to match our RAII naming conventions.
Comment 10 (dormant account) 2011-07-01 14:22:41 PDT
(In reply to comment #9)
> Would be nicer if this were AutoTimer to match our RAII naming conventions.

ok since there are two votes, I'll change it in a followup.
Comment 11 (dormant account) 2011-07-01 14:41:03 PDT
(In reply to comment #9)
> Would be nicer if this were AutoTimer to match our RAII naming conventions.

http://hg.mozilla.org/integration/mozilla-inbound/rev/0d425ab8eb13

Note You need to log in before you can comment on or make changes to this bug.