mozilla :: #content

14 Jul 2017
00:04cpearceDoes ContentParent have threadsafe refcounting?
00:06cpearceGiven I see NS_IMPL_CYCLE_COLLECTING_ADDREF(ContentParent) I guess not.
19:47erahmmystor: why don't we just call profiler_initialize_stackwalk() in the profiler init (NS_LogInit or whatever)
19:48mystorerahm: it can be really really slow (seconds), and I don't want to block the main thread during startup
19:50erahmmystor: okay I guess I need to read more context, but is it actually really slow or is it some sort of lock contention issue?
19:57erahmmystor: so presumably read_procmaps is the slow part? or are we doing something dumb like running the unit tests?
19:59erahmmystor: I am now sad I looked further :(
20:00mystorerahm: I think we read all of libxul into memory
20:00erahmmystor: Arguably it should already be in memory...
20:00mystorerahm: it ooms on 32 bit sometimes...
20:01mystorerahm: yeah. I didn't write that code so I don't know for sure.
20:01* erahm shakes fist at Julian
20:05erahmyeah wow that is going to read the entire library
20:06mystorerahm: yup~!
20:07erahmmystor: I almost want to punt this to jseward to see if we can make it less awful
20:07mystorerahm: If you want :P
20:08mystorI would kinda like to get BHR running on linux sooner rather than later, but it's not the end of the world
20:08mystorIt's been delayed for ages already and isn't exactly the most important platform to get this working on first
20:08erahmman it seems like we should be able to do a lot of this work statically at build time and just update offsets or whatever at runtime
20:09mystorerahm: I should point out that we only do this on nightly and beta, and I'm thinking we'll stop doing it on beta soon
20:09mystor(As I'm going to be increasing the amount of data we collect soon and don't want to swamp our servers with beta data)
20:10erahmmystor: Is RunMonitorThread called multiple times?
20:10mystorerahm: I believe that it can be
20:10mystorerahm: It's called in each process for sure
20:10erahmmystor: okay but not multiple per process?
20:11mystorerahm: I think it can be called multiple times per process... 1 sec lemme check
20:12mystorerahm: Nope, it looks like we have a singleton - there should only be 1
20:13erahmmystor: okay, so RunMonitorThread by definition isn't the main thread right?
20:13mystorerahm: This is true
20:13erahmmystor: I'm not sure what you're fixing then
20:14mystorerahm: I'm not really sure why moving the call off of the bhr thread helped anything
20:14mystorerahm: I think it has to do with the BHR shutdown process
20:14mystorerahm: As in with the call on the BHR thread the main thread can block on BHR during shutdown IIRC
20:15erahmmystor: ah okay, so shouldn't this just block sts shutdown then?
20:15erahmor maybe that doesn't block
20:15mystorIunno :S
20:15mystorI ran the test a few times locally and the failure stopped reproducing but I haven't finished a try push yet
20:16mystorerahm: That patch was a shot in the dark
20:16erahmmystor: and you're sure BHR is actually working with this patch? b/c my guess is mSTS is null and we just don't bother initializing and voila no hangs
20:16mystorerahm: I started the browser with the patch applied and did some stuff, then checked telemetry, and I had hangs with native stacks
20:17mystorerahm: There's a chance that the problematic processes don't have a mSTS or something and thus we dodge a bullet, but if we don't have a mSTS then we can't use native stacks anyway (as we process them on STS), so it shouldn't matter
20:17erahmmystor: That's reasonable then
20:18erahmmystor: okay review coming!
20:18mystorerahm: Sure - I won't land it unless it actually fixes the problem on try too but I'm optimistic
20:24erahmmystor: so r+ w/ some comment updates, thanks for walking me through it
20:28mystorThanks
15 Jul 2017
No messages
   
Last message: 71 days and 9 hours ago