The default bug view has changed. See this FAQ.

Measure plugin enumeration with telemetry

RESOLVED FIXED in mozilla7

Status

()

Core
Plug-ins
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: (dormant account), Unassigned)

Tracking

(Blocks: 1 bug)

unspecified
mozilla7
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(2 attachments, 2 obsolete attachments)

Comment hidden (empty)
(Reporter)

Comment 1

6 years ago
Created attachment 542953 [details] [diff] [review]
raii helper
Attachment #542953 - Flags: review?(mh+mozilla)
(Reporter)

Comment 2

6 years ago
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.
Attachment #542958 - Flags: review?(joshmoz)
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)
Attachment #542953 - Flags: review?(mh+mozilla) → review+
(Reporter)

Comment 4

6 years ago
Created attachment 543285 [details] [diff] [review]
raii helper

Template suggestion makes using this brilliant. Thanks.
Attachment #542953 - Attachment is obsolete: true
Attachment #543285 - Flags: review+
(Reporter)

Comment 5

6 years ago
Created attachment 543286 [details] [diff] [review]
telemetry for addon scanning

Updated with template change
Attachment #542958 - Attachment is obsolete: true
Attachment #543286 - Flags: review?(joshmoz)
Attachment #542958 - Flags: review?(joshmoz)
(Reporter)

Comment 6

6 years ago
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 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.
Attachment #543286 - Flags: review?(joshmoz) → review+
(Reporter)

Comment 8

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/3a5af9d36fb1
http://hg.mozilla.org/integration/mozilla-inbound/rev/4504b65f69db
Whiteboard: [inbound]
Would be nicer if this were AutoTimer to match our RAII naming conventions.
(Reporter)

Comment 10

6 years ago
(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.
(Reporter)

Comment 11

6 years ago
(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
http://hg.mozilla.org/mozilla-central/rev/4504b65f69db
http://hg.mozilla.org/mozilla-central/rev/3a5af9d36fb1
http://hg.mozilla.org/mozilla-central/rev/0d425ab8eb13
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.