Bug 1181175 - Move RDTSC to mfbt;r?froydnj,jandem

There will be a second patch once bug 1147664 has landed.

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem https://reviewboard.mozilla.org/r/12921/#review11499 I can sort of imagine what you want this in here for, but it'd be better to have you tell me than having me guess about it. :) I'd like a rationale why we shouldn't simply have the necessary code next to its use in toolkit/; I'm not sure that the "code reuse" bar in this case is quite high enough. Lots of comments about the comments; it's great that you wrote these limitations up as comments, but they need some tweaking. ::: mfbt/ClockCycles.h:21 (Diff revision 1) > + * Please trim down these large blocks of space between paragraphs/list items. One blank line is enough. ::: mfbt/ClockCycles.h:31 (Diff revision 1) > + * The only way to prevent this is to pin your process to the CPU. Nit: "pin your process to a particular CPU" Or should that be "thread" instead of "process"? ::: mfbt/ClockCycles.h:7 (Diff revision 1) > +#ifndef mozilla_ClockCycles_h Please add a one-line summary comment, as all the other headers in MFBT (should) have. ::: mfbt/ClockCycles.h:15 (Diff revision 1) > + * `rdtsc`, or Read TimeStamp Clock, is an instruction provided by Intel's own documentation calls this "Read Time-Stamp Counter", and we should follow their lead. Which also means that many, if not all, of the references to "clock" below should be rewritten into "counter" and surrounding verbiage adjusted if necessary. ::: mfbt/ClockCycles.h:50 (Diff revision 1) > + * wallclock time. The Intel documentation for processors newer than the Pentium 4 (Core Solo/Duo/2, Xeon, Atom, ...) suggests that this is not the case. "...Constant TSC behavior ensures that the duration of each clock tick is uniform and supports the use of the TSC as a wall clock timer even if the processor core changes frequency. This is the architectural behavior moving forward." So yes, we can't depend on this, but I imagine for the vast, vast majority of cases we care about, this is the behavior that we get. (It's possible my documentation is out of date, and they decided to stop doing this with newer processors.) ::: mfbt/ClockCycles.h:57 (Diff revision 1) > + * than what you expect. See [1] for a study of countermeasures. I think, given the way we implement rdtsc below, we can rule out the compiler reordering things (GCC-esque compilers use "asm volatile" or equivalent in their implementations, and I'd hope MSVC does the same thing). You may want to simply quote from the documentation: "\[rdtsc\] does not necessarily wait until all previous instructions have been executed before reading the counter. Similarly, subsequent instructions may begin execution before the read operation is performed." Or somesuch. ::: mfbt/ClockCycles.h:62 (Diff revision 1) > + * According to the web, the overhead of rdtsc is expected to be A link would be super-helpful here, even if it's only to something like Agner Fog's instruction tables. ::: mfbt/ClockCycles.h:76 (Diff revision 1) > +#include <intrin.h> Let's dispense with all these separate cases and write simply (albeit with lots of preprocessor testing): // Possibly better to use the \_M_IX86 and\\\_M_X64 macros here. \#if defined(\_WIN32) || defined(\_\_i386\_\_) || defined(\_\_x86_64\_\_) \#include <x86intrin.h> static inline uint64_t ReadTimestampCounter(void) \{ return \_\_rdtsc(); \} \#define MOZ_HAVE_RDTSC 1 \#endif The \_\_powerpc\_\_ block should stay in js/, I think. If somebody \*really\* needs it in MFBT, then we can talk about that; I'd want to do some research on whether the time base registers on PPC are subject to the same limitations as rdtsc on x86. ::: toolkit/components/perfmonitoring/nsPerformanceStats.cpp:25 (Diff revision 1) > -#include "windows.h" > +#include "Windows.h" This file looks like a bunch of changes that I might be the correct reviewer for...but I have a hard time seeing how they are tied to this bug. It's also difficult to review these changes without a clear rationale for them. Please resubmit. ::: mfbt/ClockCycles.h:25 (Diff revision 1) > + * hibernating, single CPU going to full sleep). Moreover, on I am unconvinced that the counter resets for hibernation or full sleep; the documentation for the counter certainly suggests otherwise (e.g. "The RDTSC instruction...is guaranteed to return a monotonically increasing unique value whenever executed \[excluding wraparound\]...Intel guarantees that the time-stamp counter will not wraparound within 10 years after being reset". I think you want to scrap this line about the reset behavior; maybe you want to say something about counters on individual cores being monotonic, but given OS scheduling, observed results may be different in the process migrates across CPUs? ::: mfbt/ClockCycles.h:38 (Diff revision 1) > + * regardless of the process that has issued this instruction. Moreover, I can't find anything in the Intel documentation about exactly when the counter increments, other than some hand-waviness around "processor events". So maybe something like: "The counter increases at all times, not just when the particular process you want to measure is running on a particular CPU. Due to scheduling decisions, this means that the measured time between two rdtsc operations may vary significantly over several runs of the same code." Since we're not running in a kernel context, there's really nothing we can do about ensuring we have exclusive ownership of the CPU.

https://reviewboard.mozilla.org/r/12921/#review11499 It's not used in toolkit/, afaict. It's used in js/, which cannot depend on toolkit/. As it turns out it's already used by two different modules: TraceLogger, and with my patch, PerformanceStats. No, I could just move it to jsinlines.h, but I figured it would do more good in mfbt/. Your call. > I am unconvinced that the counter resets for hibernation or full sleep; the documentation for the counter certainly suggests otherwise (e.g. "The RDTSC instruction...is guaranteed to return a monotonically increasing unique value whenever executed \[excluding wraparound\]...Intel guarantees that the time-stamp counter will not wraparound within 10 years after being reset". > > I think you want to scrap this line about the reset behavior; maybe you want to say something about counters on individual cores being monotonic, but given OS scheduling, observed results may be different in the process migrates across CPUs? Well, the OS does turn off the CPU when the computer is hibernating. I also remember reading that some OSes can turn off individual CPUs, although I haven't really investiated this. In either case, I'm 99% sure that turning off a CPU resets the counter of that CPU to 0. > I can't find anything in the Intel documentation about exactly when the counter increments, other than some hand-waviness around "processor events". So maybe something like: > > "The counter increases at all times, not just when the particular process you want to measure is running on a particular CPU. Due to scheduling decisions, this means that the measured time between two rdtsc operations may vary significantly over several runs of the same code." > > Since we're not running in a kernel context, there's really nothing we can do about ensuring we have exclusive ownership of the CPU. > I can't find anything in the Intel documentation about exactly when the counter increments, other than some hand-waviness around "processor events". The link I put at the end of the doc, which is an Intel paper, assumes that the only way to get reliable readings is to be the only process on the CPU. But fair enough. > The Intel documentation for processors newer than the Pentium 4 (Core Solo/Duo/2, Xeon, Atom, ...) suggests that this is not the case. "...Constant TSC behavior ensures that the duration of each clock tick is uniform and supports the use of the TSC as a wall clock timer even if the processor core changes frequency. This is the architectural behavior moving forward." > > So yes, we can't depend on this, but I imagine for the vast, vast majority of cases we care about, this is the behavior that we get. > > (It's possible my documentation is out of date, and they decided to stop doing this with newer processors.) No, you are right, I based that warning on older architectures. I forget the version number. > A link would be super-helpful here, even if it's only to something like Agner Fog's instruction tables. Unfortunately, I couldn't find that link. I'll try again. > Let's dispense with all these separate cases and write simply (albeit with lots of preprocessor testing): > > // Possibly better to use the \_M_IX86 and\\\_M_X64 macros here. > \#if defined(\_WIN32) || defined(\_\_i386\_\_) || defined(\_\_x86_64\_\_) > \#include <x86intrin.h> > static inline uint64_t > ReadTimestampCounter(void) > \{ > return \_\_rdtsc(); > \} > > \#define MOZ_HAVE_RDTSC 1 > \#endif > > The \_\_powerpc\_\_ block should stay in js/, I think. If somebody \*really\* needs it in MFBT, then we can talk about that; I'd want to do some research on whether the time base registers on PPC are subject to the same limitations as rdtsc on x86. Frankly, with the exception of the `MOZ_HAVE_RDTSC` macro, I just moved code around. Is `x86intrin.h` standard? > This file looks like a bunch of changes that I might be the correct reviewer for...but I have a hard time seeing how they are tied to this bug. It's also difficult to review these changes without a clear rationale for them. Please resubmit. Er... Commit error. Shouldn't have been there.

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Moving RDTSC and prmjtime.h to Time.h;r?nbp

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem I have no idea why MozReview is not publishing my review … in the meantime … Undo the removal of powerpc support, and r=nbp.

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Moving RDTSC and prmjtime.h to Time.h;r=nbp

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Moving RDTSC and prmjtime.h to Time.h;r=nbp

Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r?jandem

https://reviewboard.mozilla.org/r/12919/#review12379 Ok, this time, attaching to the right bug.

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #9) > Bug 1181175 - Use RDTSC for Performance Monitoring instead of > getrusage;r?jandem Sorry but this patch seems to make a lot of (unrelated) changes? Would it be possible to split it up in smaller parts? Without a description of the patch, it's really hard to review.

I realize that the change is complex, sorry about that. Unfortunately, I don't really see how I could split the changes into self-consistent patches. I'll do what I can, but in the meantime, let me give an overview of the patch. The objective is to replace most calls to `getrusage` & co. in the code, as they have the following drawbacks: 1/ they are not necessarily monotonic, which contributes to sometimes weird results; 2/ they are designed for 10ms-scale benchmarks, while we use them for much shorter intervals; 3/ they take lots of time. With this patch, we rather use `rdtsc`, which returns the number of clock cycles elapsed since CPU start. Within a number of hypothesis (documented in Runtime.cpp), we hope that this change will let us limit considerably the errors we get by using `getrusage` for short intervals, and make the code much faster. The new algorithm goes as follows: - we only compute `getrusage` twice, once when we start executing JS code in the event, and once when the event is complete; - AutoStopwatch now collects the number of clock cycles elapsed while executing the content of the PerformanceGroup, and stores them in a temporary value `recentCycles` that will only be committed once we have reached the end of the event; - same thing for `recentCPOW` and `recentTicks`; - once we have reached the end of the event, we compute the delta of `getrusage` between the start of JS and the end of the event, and we use the number of clock cycles spent in each PerformanceGroup to determine which proportion of this delta should be attributed to each compartment. That's most of the change. Somewhere along the way, I needed to refactor refcounting of `PerformanceGroup` to be able to maintain a list of "dirty" PerformanceGroups. Is this helpful? Do you want me to detail what changed in each file?

https://reviewboard.mozilla.org/r/12919/#review12379 I found a way to subdivide the patch into two smaller patches. Theoretically, the RefPtr patch could be landed before the RDTSC patch. I hope this helps with the review.

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Moving RDTSC and prmjtime.h to Time.h;r=nbp

Bug 1181175 - PerformanceGroup now uses mozilla::RefPtr;r?jandem

Comment on attachment 8637153 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r?jandem

Comment on attachment 8639523 [details] MozReview Request: Bug 1181175 - PerformanceGroup now uses mozilla::RefPtr;r=jandem https://reviewboard.mozilla.org/r/14211/#review12953 ::: js/src/vm/Interpreter.cpp:446 (Diff revision 1) > - } > + } Can drop the {} now that it's a single line body, also above. ::: js/src/vm/Interpreter.cpp:484 (Diff revision 1) > + { { should go on the previous line, as it was before. ::: js/src/vm/Interpreter.cpp:500 (Diff revision 1) > - void exit() { > + void exit() Same here and below. ::: js/src/vm/Interpreter.cpp:562 (Diff revision 1) > + sharedGroup_); Hm these 4 lines should fit on a single line, or else two lines. Same below. ::: js/src/vm/Interpreter.cpp:579 (Diff revision 1) > + } Nit: no {} ::: js/src/vm/Runtime.cpp:979 (Diff revision 1) > + , runtime_(rt) Nit: SpiderMonkey style has the commas at the end of the line, also below.

Comment on attachment 8637153 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem https://reviewboard.mozilla.org/r/13817/#review12951 This seems okay for now, but if this becomes more complicated I really think we should take a step back and look for ways to simplify it. ::: js/src/jsapi.h:5641 (Diff revision 2) > -ResetStopwatches(JSRuntime*); > +FlushMonitoring(JSRuntime*); Nit: "FlushPerformanceMonitoring" and "ResetPerformanceMonitoring" seem a bit more precise. ::: js/src/vm/Interpreter.cpp:520 (Diff revision 2) > - // We make no attempt to recover from this error. If > + // CPU as we started. Note that we can be rescheduled to Nit: "on" the same CPU. Discarding results in this case is a bit unfortunate; maybe you could do some measurements to see how common this is on different loads and different platforms. ::: js/src/vm/Interpreter.cpp:397 (Diff revision 2) > - uint64_t userTimeStart_; > + uint64_t cyclesStart_; There's more and more code here; at some point we should probably move it into vm/PerformanceMonitoring or something. Or a better name that avoids confusion with the profiler code. ::: js/src/vm/Runtime.cpp:915 (Diff revision 2) > +// Nit: remove these // lines. ::: js/src/vm/Runtime.cpp:926 (Diff revision 2) > +// or core with an unsynchronized clock; Nit: the indentation here is a bit odd; add one more space before "or core", also below. ::: js/src/vm/Runtime.cpp:933 (Diff revision 2) > +// Nit: 4 'blank' lines here seems a bit much, also below. ::: js/src/vm/Runtime.cpp:938 (Diff revision 2) > +// the effect will be contained to a single iteration in the event loop. Nit: s/in/of/ ? ::: js/src/vm/Runtime.cpp:993 (Diff revision 2) > +// Nit: remove these 'blank' lines at the end.

Yes, thanks for splitting this up and sorry for the delay!

(In reply to Jan de Mooij [:jandem] from comment #19) > Yes, thanks for splitting this up and sorry for the delay! (Oh and I feel bad about this, but I *strongly* dislike the Review Board UI and that's one of the reasons I keep delaying reviews there. I should really talk to them though..)

https://reviewboard.mozilla.org/r/13817/#review12951 > There's more and more code here; at some point we should probably move it into vm/PerformanceMonitoring or something. > > Or a better name that avoids confusion with the profiler code. Fair enough. Do you want to do this now or in a followup bug? > Nit: "on" the same CPU. > > Discarding results in this case is a bit unfortunate; maybe you could do some measurements to see how common this is on different loads and different platforms. Good point, if we could avoid discarding results, this would simplify the code nicely (and probably make it faster). I suppose I could gather some data through Telemetry, although this would probably complicate the code of `Runtime::Stopwatch::commit()`. Do you suggest I do something along these lines, or do you have another idea?

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Moving RDTSC and prmjtime.h to Time.h;r=nbp

Comment on attachment 8639523 [details] MozReview Request: Bug 1181175 - PerformanceGroup now uses mozilla::RefPtr;r=jandem Bug 1181175 - PerformanceGroup now uses mozilla::RefPtr;r=jandem

Comment on attachment 8637153 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r?jandem

Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r?jandem,bsmedberg

Comment on attachment 8640634 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Assuming that this is about data collection review of the histogram, f+

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #21) > Fair enough. Do you want to do this now or in a followup bug? Followup is fine. > Good point, if we could avoid discarding results, this would simplify the > code nicely (and probably make it faster). I suppose I could gather some > data through Telemetry, although this would probably complicate the code of > `Runtime::Stopwatch::commit()`. > > Do you suggest I do something along these lines, or do you have another idea? Hm Telemetry might be nice, but can you use the runtime->addTelemetry callback there? That's how we usually get telemetry data from JS. We could probably call that in ~JSRuntime, or somewhere else if that's too late.

https://reviewboard.mozilla.org/r/13817/#review12951 > Good point, if we could avoid discarding results, this would simplify the code nicely (and probably make it faster). I suppose I could gather some data through Telemetry, although this would probably complicate the code of `Runtime::Stopwatch::commit()`. > > Do you suggest I do something along these lines, or do you have another idea? Calling `addTelemetry` from `AutoStopwatch` sounds a bit scary, as it can be called dozens of times per event. The patch I just posted for review (well, more feedback) delegates the telemetry calls to the `nsPerformanceStatsService`, which fetches the data every few seconds.

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Moving RDTSC and prmjtime.h to Time.h;r=nbp

Comment on attachment 8639523 [details] MozReview Request: Bug 1181175 - PerformanceGroup now uses mozilla::RefPtr;r=jandem Bug 1181175 - PerformanceGroup now uses mozilla::RefPtr;r=jandem

Comment on attachment 8637153 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem

Comment on attachment 8640634 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r?jandem,bsmedberg

Jandem, do you wish to perform the Telemetry investigation in this bug or a followup?

Comment on attachment 8640634 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg https://reviewboard.mozilla.org/r/14353/#review13133 r=me with comments below addressed. ::: js/src/jsapi.h:17 (Diff revision 2) > +#endif // defined(XP_WIN) || defined(XP_LINUX) Let's remove the macro and just test for XP_WIN or XP_LINUX where we increment the counters. Then when we submit the data we can check if either counter is non-zero. We try to avoid such #ifdefs because it can easily lead to platform-specific build bustage. ::: js/src/vm/Runtime.h:1698 (Diff revision 2) > + : stayed(0), > + moved(0) Nit: indent these with 2 spaces less. ::: toolkit/components/perfmonitoring/nsPerformanceStats.cpp:507 (Diff revision 2) > + const uint32_t proportion = ( 100 * moved ) / (stayed + moved); Make sure stayed + moved != 0 to avoid a division by zero. There's also inconsistent spacing between ( and ).

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #33) > Jandem, do you wish to perform the Telemetry investigation in this bug or a > followup? This bug is fine I think. (In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #28) > Calling `addTelemetry` from `AutoStopwatch` sounds a bit scary, as it can be > called dozens of times per event. Well that's why I suggested calling it from ~JSRuntime instead of AutoStopwatch :) But this approach also works.

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Moving RDTSC and prmjtime.h to Time.h;r=nbp

Comment on attachment 8639523 [details] MozReview Request: Bug 1181175 - PerformanceGroup now uses mozilla::RefPtr;r=jandem Bug 1181175 - PerformanceGroup now uses mozilla::RefPtr;r=jandem

Comment on attachment 8637153 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem

Comment on attachment 8640634 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg

Joel, when this bug lands, it should have an impact on performance. I expect one of the following scenarios: 1/ improvements on all platforms; 2/ improvements on MacOS X, stagnation or decrease on Linux and Windows; 3/ decrease everywhere; 4/ no visible change. Can you keep an eye on it? If it's 1/ or 4/, we clearly want to keep the patch. If it's 2/, we may want to back it out after a few days (we need to gather some Telemetry) and fix it. If it's 3/, of course, we're not so happy.

Comment on attachment 8637153 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem

Comment on attachment 8640634 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Moving RDTSC and prmjtime.h to Time.h;r=nbp

Comment on attachment 8639523 [details] MozReview Request: Bug 1181175 - PerformanceGroup now uses mozilla::RefPtr;r=jandem Bug 1181175 - PerformanceGroup now uses mozilla::RefPtr;r=jandem

Comment on attachment 8637153 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem

Comment on attachment 8640634 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg

thanks for the heads up! I look forward to this landing and will keep an eye on it.

To simplify landing, I have split this bug in 3 parts.

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Moving RDTSC and prmjtime.h to Time.h;r=nbp

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Moving RDTSC and prmjtime.h to Time.h;r=nbp

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem

Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem

Comment on attachment 8644304 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem

Comment on attachment 8644304 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Bug 1147348 - Expose the timestamp of the latest PerformanceStats measure (low-level);r?jandem

Comment on attachment 8644304 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem

Comment on attachment 8644304 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg

Comment on attachment 8644304 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d6e6cbb9af20

https://hg.mozilla.org/integration/fx-team/rev/0a92886f497a https://hg.mozilla.org/integration/fx-team/rev/21b660154b4a

Backed out for B2G bustage. https://treeherder.mozilla.org/logviewer.html#?job_id=4129077&repo=fx-team /home/worker/workspace/gecko/js/src/vm/Runtime.cpp:1089:47: error: 'ArrayLength' was not declared in this scope https://hg.mozilla.org/integration/fx-team/rev/3ccebe8a8e61

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem

Comment on attachment 8644304 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9b6fb9d3d28

https://hg.mozilla.org/integration/mozilla-inbound/rev/de9ae2ccb73b https://hg.mozilla.org/integration/mozilla-inbound/rev/db4294fb662d

Backed out for Android test_compartments.js failures. https://treeherder.mozilla.org/logviewer.html#?job_id=12701411&repo=mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/9e01460eca0c

Nooooooooo :(

Mmmh... Now that the algorithm is probabilistic, that test actually doesn't make sense anymore. I'll deactivate it and think of a better test.

Bug 1181175 - Get rid of test_compartments.js;r=yoric

Sorry, this needs to be rebased now :(

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem

Comment on attachment 8644304 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg

Comment on attachment 8645955 [details] MozReview Request: Bug 1181175 - Get rid of test_compartments.js;r=yoric Bug 1181175 - Get rid of test_compartments.js;r=yoric

Rebased.

Just in case, Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=640a775506ed

applying 49d2903e5f0b patching file js/xpconnect/src/nsXPConnect.cpp Hunk #1 FAILED at 942 Hunk #2 FAILED at 990 2 out of 2 hunks FAILED -- saving rejects to file js/xpconnect/src/nsXPConnect.cpp.rej patching file js/xpconnect/src/xpcprivate.h Hunk #1 FAILED at 609 1 out of 1 hunks FAILED -- saving rejects to file js/xpconnect/src/xpcprivate.h.rej patch failed to apply :(

Oh, great, the last batch of changes actually ruins my patch :/

Comment on attachment 8631511 [details] MozReview Request: Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem Bug 1181175 - Use RDTSC for Performance Monitoring instead of getrusage;r=jandem

Comment on attachment 8644304 [details] MozReview Request: Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg Bug 1181175 - Telemetry for finding out how often our process is rescheduled to another CPU;r=jandem,bsmedberg

Comment on attachment 8645955 [details] MozReview Request: Bug 1181175 - Get rid of test_compartments.js;r=yoric Bug 1181175 - Get rid of test_compartments.js;r=yoric

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a254c0d1eb1b

https://hg.mozilla.org/integration/mozilla-inbound/rev/38ac1237696c https://hg.mozilla.org/integration/mozilla-inbound/rev/81fdd93f436f https://hg.mozilla.org/integration/mozilla-inbound/rev/defee685fedd

https://hg.mozilla.org/mozilla-central/rev/38ac1237696c https://hg.mozilla.org/mozilla-central/rev/81fdd93f436f https://hg.mozilla.org/mozilla-central/rev/defee685fedd

\o/