128673 - jprof should use Linux's /dev/rtc for up-to-8KHz sampling

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Mike Shaver (:shaver emeritus)]

We can, with some work, get the kernel to give us a signal for every RTC
interrupt, and that knob goes to 8192Hz.

We need to patch the Linux kernel's /dev/rtc so that we can crank that knob
without requiring Mozilla to run as root.  I have that patch, and have submitted
it for general integration.

Then we need to make jprof use the RTC.  I have a sample app that I used to test
the RTC facilities, which I will also attach.  The jprof bits don't look too hard.

Comment 1

[Mike Shaver (:shaver emeritus)]

Attachment: Patch to allow configuration of non-privileged RTC interrupt frequency ceiling.

Apply in linux/drivers/char and rebuild (just the rtc.o module if yours is
modular -- Red Hat kernels default to non-modular).  You will need to make sure
that RTC_IRQ is set as well, though I think it's the default on systems that
support it.

You can then do this as root, to allow user-space apps to use frequencies up to
8192Hz:

# echo 8192 > /proc/sys/dev/rtc/max-user-freq

Comment 2

[Mike Shaver (:shaver emeritus)]

Attachment: Sample program demonstrating use of /dev/rtc for high-frequency signal generation

Requires the previous source patch, unless you just want to run it as root. 
(That's no fun!)

From one of my runs at 8192 Hz (essentially-idle Athlon 1.4GHz):
9999 intervals, min interval 5 usec, max 329 usec, avg 122.083508

I should probably add std-dev, once I remember how to calculate it.

Comment 3

[Mike Shaver (:shaver emeritus)]

But now I find myself asking: if we want jprof to do sub-millisecond sample
intervals, why are we placing a lower bound on it explicitly in setupProfilingStuff?

dbaron, did you say something before about setitimer not working for < 1ms
intervals?

Comment 4

[Mike Shaver (:shaver emeritus)]

Ah, you need non-default HZ in your kernel to do sub-ms profiling with
setitimer.    I'll go back to my jprof hacking now.

Comment 5

[Mike Shaver (:shaver emeritus)]

Performance is king.

Comment 6

[jim_nance]

Great idea Mike!

Comment 7

[Mike Shaver (:shaver emeritus)]

Attachment: Fix mark I: JP_DEFER doesn't work.

I can use JPROF_FLAGS="JP_START JP_RTC_HZ=4096" to generate a log that looks to
have the right number of samples.

JP_DEFER doesn't work, because right now I don't handle SIGPROF at all in the
JP_RTC case, and that leaves it with its default (fatal!) disposition.	I know
how I'm going to fix it, but I wanted to get this patch up soon for testing and
review: the jprof code is a bit hairy, and I'm not sure I understand all the
interactions between the global variables, etc.

Comment 8

[Mike Shaver (:shaver emeritus)]

Attachment: Fix mark II: JP_DEFER works, things are looking good

Looks good so far. Have to use SIGIO to trigger profile-start if you're using
the RTC, but that doesn't bother me too much.

I'm getting ~36000 hits on a 3 second profile, so I think at least some math is
off.  Have to dig deeper, maybe I'm not suppressing the itimer stuff?  But I
can profile, and get really big logs at 8192Hz!

Comment 9

[Mike Shaver (:shaver emeritus)]

This isn't worth plopping on the 1.0 branch, IMO.  Out to 1.1alpha, I'll fish
for reviews soon.

Comment 10

[Brian Ryner (not reading)]

Attachment: slightly improved patch

This fixes a few random problems I observed running the last patch:

- Only collect data on the main thread (SIGIO gets sent to all threads) since
our stack crawl isn't threadsafe.
- Fix the 'skip' count to correspond to what I observed to be the correct
number of frames to skip.  I tested all the way back to RedHat 6.2 and the
number of stack frames we need to skip didn't change.
- Sanity-check the EIP value we get from the signal handler and throw it out if
it's not usable (we can still get the rest of the stack anyway)

Comment 11

[Mike Shaver (:shaver emeritus)]

Comment on attachment 134273 [details] [diff] [review]
slightly improved patch

Righteous, as I knew it would be. r=shaver.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 134273 [details] [diff] [review]
slightly improved patch

>+                if (!IS_POWER_OF_TWO(rtcHz) || rtcHz < 2) {
>+                    fprintf(stderr, "JP_RTC_HZ must be power of two and > 2, "

Slight disagreement here about whether 2 is OK.

writeStrStdout was replaced with a mix of puts and write -- why the difference?
 And it looks like you removed the "\n" from the first signal text.

sr=dbaron

Comment 13

[Brian Ryner (not reading)]

checked in (I changed all of the stdout printing to use puts).

Comment 14

[Boris Zbarsky [:bzbarsky]]

So if I understand this patch correctly, using JP_RTC_HZ automatically implies
JP_REALTIME?

Also, stopping the profile with SIGUSR1 does not seem to work when using
JP_RTC_HZ -- the reason is that the timer is stopped by calling
startSignalCounter(0) in DumpAddressMap(), which naturally has no effect
whatsoever on the RTC stuff.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch to make pausing work

I tested this by the simple method of running a profile for 5 seconds, then
pausing it, then waiting 30 seconds before quitting Mozilla.  Without this
patch, at 64Hz I was ending up with around 2000 hits in the profile; with the
patch I get a bit over 300.

Comment 16

[Mike Shaver (:shaver emeritus)]

Comment on attachment 143092 [details] [diff] [review]
Patch to make pausing work

Yeah, that looks fine.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 143092 [details] [diff] [review]
Patch to make pausing work

fix the perror that still says "| F_ASYNC" and sr=dbaron

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 143092 [details] [diff] [review]
Patch to make pausing work

Did that and checked in.