Closed Bug 457949 Opened 12 years ago Closed 11 years ago

no need for Stopwatch

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: taras.mozilla, Assigned: taras.mozilla)

References

Details

Attachments

(3 files)

Stopwatch is only used when MOZ_PERF_METRICS is specified. It's unclear if anyone still uses that. For now I made stopwatch conditional on MOZ_PERF_METRICS being set.

From the CVS log looks like nobody has touched the code in half a decade. Asking Benjamin as he seems to be the person to talk to for abandoned stuff in the tree.
Attachment #341174 - Flags: review?(benjamin)
Attachment #341174 - Flags: review?(benjamin) → review-
Comment on attachment 341174 [details] [diff] [review]
don't compile stopwatch by default

You should stop building stopwatch.cpp by removing it from CPPSRCS in the makefile, rather that ifdefing the entire file.
if we are going to be removing files from compilation, the correct fix would be to not compile libutil unless MOZ_PERF_METRICS is set. Could you hint at how to do that?
Actually looks like --enable-perf-metrics is broken:

make[3]: Entering directory `/home/taras/work/ff-build.fennec/xulrunner/toolkit/library'
make[3]: *** No rule to make target `-lmozutil_s', needed by `libxul.so'.  Stop.

Not sure how long it's been broken for.
when would be a good time to land a nuking of --enable-perf-metrics?
So, MailNews is now interested in using the stopwatch functionality, tentatively exposed through a thin C++ XPCOM wrapper.  How would people feel about changing things up so that MailNews projects (Thunderbird and SeaMonkey) could enable this mechanism and the wrapper?  Since this would not affect Firefox (other than the general disabling, but we could avoid changing that for now), we might perchance get it into 1.9.1?  (If it can't make 1.9.1, we would need to just copy it into comm-central, barring a better solution to our actual problem.)

The actual problem is a need for the MailNews Global Database (gloda) to be adaptive about its indexing so it doesn't consume 100% of the user's CPU.  We can't simply bracket our invocations of functionality with timestamp start/stop's (not that that is a great option) because there is a lot of asynchronous logic happening un-bracketed by javascript (libmime streaming, asynchronous SQLite logic), etc.  The prototype implementation using Stopwatch uses the exposed CPU Time and real clock time (samplings spaced by nsITimer events) to estimate Thunderbird's CPU utilization over a reasonable time interval.  We set target CPU utilization thresholds based on whether the user is idle (using the idle service) and twiddle our knobs until we are in the target range.

I'm not sure how else I would really go about doing this without using Stopwatch or writing a new cross-platform equivalent, but I'm certainly open to suggestions.  Feedback on whether this is desirable in mozilla-central and the best way to do it is appreciated.
Is it possible to move stopwatch code to mailnews?
(In reply to comment #7)
> Is it possible to move stopwatch code to mailnews?

From my perspective, sure.  As long as there's nothing in mozilla-central that needs access to it, it could move into comm-central/mailnews (or some other top-level comm-central directory) without trouble.

Such a strategy also would not need to do anything drastic for 1.9.1; the removal of Stopwatch could take place on the trunk.

The argument against, in my mind, would be that such code has more in common with the platform found in mozilla-central than comm-central.  But that's tenuous, as it's hard to imagine what other code would need this functionality.  (Especially since other programs would hopefully not be in the boat Thunderbird is in where a lot of things have to be time-shared on the main thread owing to serious single-threaded legacy code.)
(In reply to comment #8)
> (In reply to comment #7)
> > Is it possible to move stopwatch code to mailnews?
> 
> From my perspective, sure.  As long as there's nothing in mozilla-central that
> needs access to it, it could move into comm-central/mailnews (or some other
> top-level comm-central directory) without trouble.

Sounds good.

> Such a strategy also would not need to do anything drastic for 1.9.1; the
> removal of Stopwatch could take place on the trunk.

It wont happen in 191.
> 
> The argument against, in my mind, would be that such code has more in common
> with the platform found in mozilla-central than comm-central.  But that's
> tenuous, as it's hard to imagine what other code would need this functionality.

Yeah it is not a good fit for measuring things in firefox. I think we are moving away from having non-firefox components in mozilla central in general.
Lets get this in for next release since I missed the boat for 3.5
Attachment #395667 - Flags: review?(benjamin)
Attachment #395667 - Flags: review?(benjamin) → review+
http://hg.mozilla.org/mozilla-central/rev/092a301bd484
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Assignee: nobody → tglek
Flags: in-testsuite-
OS: Linux → All
Hardware: Other → All
Target Milestone: --- → mozilla1.9.3a1
Depends on: 546103
Comment on attachment 426929 [details] [diff] [review]
(Cv1-CC) Remove MOZ_PERF_METRICS from comm-central too (on 1.9.2+)
[Checkin: Comment 15]

Andrew, the review request here is just to make sure mailnews doesn't need these bits in any "coming" work.  (Based on earlier comments).  The patch itself is good otherwise.
Attachment #426929 - Flags: review?(bugspam.Callek)
Attachment #426929 - Flags: review?(bugmail)
Attachment #426929 - Flags: review+
Attachment #426929 - Flags: review?(bugmail) → review+
Comment on attachment 426929 [details] [diff] [review]
(Cv1-CC) Remove MOZ_PERF_METRICS from comm-central too (on 1.9.2+)
[Checkin: Comment 15]

We migrated/ported nsStopwatch into mailnews/base/util per the proposal in the thread.  Remove away!
Comment on attachment 426929 [details] [diff] [review]
(Cv1-CC) Remove MOZ_PERF_METRICS from comm-central too (on 1.9.2+)
[Checkin: Comment 15]


http://hg.mozilla.org/comm-central/rev/903c24461dd5
Attachment #426929 - Attachment description: (Cv1) Remove MOZ_PERF_METRICS from comm-central too (on 1.9.2+) → (Cv1-CC) Remove MOZ_PERF_METRICS from comm-central too (on 1.9.2+) [Checkin: Comment 15]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.