mozilla :: #jsapi

14 Jul 2017
00:00mrgigglesthe mozilla-inbound tree is now open
07:59mrgigglesthe mozilla-inbound tree is now closed (windows2012 static faiure; gl failures and unexpected passes)
08:15mrgigglesthe mozilla-inbound tree is now open
10:26smaugjonco: curious, what does FULL_STORE_BUFFER mean?
10:28joncosmaug: we keep track of things that point into the nursery in the store buffer, so we don't have to mark all roots when doing a minor collection
10:28joncoreally it's made up of several buffers for different kinds of pointer
10:29joncothere's a somewhat arbitrary limit on how large these can grow before we trigger a minor collection
10:29joncoin this case I think it's pointers from the browser heap to nursery allocated objects that is causing this
10:29smaugjonco: where is that limit defined? Would it make sense to try to increase the limit?
10:29joncoyes
10:30joncoMaxEntries in js/src/gc/StoreBuffer.h
10:30joncothe current value allows a size of ~6000 entries
10:30smaugthat is not very much
10:31joncono, but we also want to keep the minor GC time short
10:31joncoand we have check all these entries when we do
10:31smaugJS engine has rather interesting convention to never hint what the actual limits are :)
10:32smauglike here, it depends on the sizeof T
10:32smaugnursery size depends on sizeof some chunks
10:32joncoyeah that is not super helpful
10:33smaugbut ok, I'll play a bit with MaxEntries.
10:33joncosmaug: great
10:38jandemi still think it would be interesting to understand what these entries are
10:39jandemmaybe it's unavoidable and bumping the limit is fine, maybe there's an underlying issue that's worth fixing
10:41smaugjandem: yeah, would be great if someone who understands GC more could figure out if we could improve GC handling while running speedometer
10:42smaugwe trigger minorGC very often, and also majorGC slices from the interrupt callback thingie
10:49joncojandem: they are recording pointers into the nursery from (in this case) the browser heap
10:50jandemjonco: i know but don't we deduplicate them? Or is this buffer keyed on the source address?
10:51jonco jandem: yes we need to keep track of the source address so we can update the pointer
10:51joncojandem: I can have a look and see what's creating these
10:52jandemjonco: so these are Heap<> things you think?
10:52joncoyes
10:53jandemjonco: ok, just curious
10:53joncoI&#39;ll have a look after lunch
10:53joncoI expect the test is just creating a bunch of DOM stuff
10:53joncobut there could be something we can improve
10:54jandemjonco: cool, thanks
11:20pboneEvening.
11:53joncoevening pbone
12:19pbonejonco: Hi, I&#39;ve successfully got a _tiny_ bit more GC information into the gecko profiler.
12:19pboneand sfinks json tool is _really_ cool.
12:20pboneI was just starting to get back to figuring out my problem. When I noticed another feature I&#39;d like and got myself distracted starting to implement that.
12:20pbonea VERY useful feature when you do profiling, is to be able to name the profiles and have this reflected in the UI. SO you know which is which.
12:21joncook that sounds useful yes
12:21pboneBut it was a bit of a distraction :-(
12:21joncoI think sfink is working adding more profiling information for minor GCs
12:22pboneI saw that.
12:23pbonethey only have a tiny amount of detail. I noticed that also.
12:23pbonebut the major GCs and their slices have all the info from Statistics.cpp
12:23joncook, and we can expose this now?
12:24pboneYes. The trick is to know how, how to make it look resonable in the GUI.
12:24joncogreat
12:24pboneMy first change is to put the gc reason in a tooltip.
12:24pbonenext I want to see incremental / non-incremental and for each slice also.
12:25joncoyeah, I&#39;d be happy with just having everything into a tooltop
12:25pbonebut I don&#39;t know where to show that.
12:25joncoin*
12:25joncotooltip*
12:25pboneThen I want to see what phases a GC was doing, then their timings.
12:25joncoright
12:26pboneafter that I don&#39;t knowl
12:26pboneAlthough, Statistics.cpp only records the phases&#39; durations.
12:26pbonenot their start/end times.
12:26pboneso we can&#39;t plot them on the timeline without making some assumptions, and I don&#39;t want to make assumptions for this kind of thing.
12:27pboneWe could add that, but maybe later.
12:27joncoStatistics::SliceData has |TimeStamp start, end|
12:27joncoalthough that may not get exposed
12:27pboneNo, I can see that already.
12:27pbonemostly.
12:28pboneI&#39;m going to have to wait until tuesday to talk to mstrange about that.
12:28pboneI&#39;ll write an e-mail
12:28joncook
12:28pboneWe see them plotted based on start/end times, but in the list view only the start time is visible. I&#39;d like to see both there.
12:28pboneso will need to ask.
12:28joncook
12:28pboneI tried to find it myself already.
12:29pboneBut what I mean is, at what time did it start marking, then what time did it finish marking, when did it start sweeping. etc.
12:29pbonethose are only durations.
12:30joncooh right, that&#39;s correct
12:30joncowe only record aggregate durations for each category of work
12:31pboneYeah, I&#39;d like to see how it switches between them.
12:31pboneI don&#39;t know if that&#39;ll be useful, mayber.
12:31pbonemaybe.
12:31joncoI&#39;m not sure how useful that&#39;s going to be
12:31joncowe may move between different phases many times in one slice
12:33pboneyeah, that could be too noisy, or could give us some insight we weren&#39;t expecting.
12:38evilpieDeclarationKind doesn&#39;t seem to have a way to check for classes? I think fixing bug 1282431 would be good
12:38firebothttps://bugzil.la/1282431 NEW, nobody@mozilla.org Class redeclaration error message mentions &quot;let&quot; as cause
12:40joncojandem: what&#39;s the best way to run speedometer v2? the link from bug 1339557 is gives a 404
12:40firebothttps://bugzil.la/1339557 NEW, nobody@mozilla.org [Meta] Quantum Release Criteria: Figure out why we are so slow on Speedometer v2
12:41jandemjonco: I use http://speedometer2.benj.me/ - I think that&#39;s what we have on AWFY
12:41jandemor http://speedometer2.benj.me/InteractiveRunner.html
12:42jandemyou can add ?suite=... to run a particular framework subtest
12:42joncojandem: great, thanks
12:42jandemnp
12:50bbouviernote webkit also hosts it on their website: http://browserbench.org/Speedometer/
12:53joncobbouvier: that&#39;s v1
12:53bbouvierjonco, no, it&#39;s saying v1 but it&#39;s v2
12:53bbouvieri&#39;ve opened a pull request for that on github, but they use a different bug tracker
12:53joncoaha
12:53joncobbouvier: thank you :)
12:53bbouvierthe proof it&#39;s v2 is on their commit history
12:54joncoI found that but thought it was an old version
13:20pbonejonco
13:21pbonethe first thing to notice about the weird behaviour in my workspace is some really long GCMinor puases.
13:22joncopbone: like how long? we aim to keep them below 1 mS
13:23pbone58ms
13:23pboneit&#39;s really broken.
13:23jonco(if you&#39;re benchmarking you should run with JSGC_DISABLE_POISONING=1 but I doubt it&#39;s that)
13:23joncooh, that&#39;s bad
13:23pbonethat&#39;s just my workspace, tho, not committed.
13:23joncothis is an opt build?
13:24pboneI guess, whatever ./mach build does
13:24pboneTHere&#39;s some stuff in my moz_config, but that&#39;s ccache and --enable-profiling
13:25joncook, I&#39;m pretty sure enable-optimize and disable-debug are defaults
13:25joncocan you see what it&#39;s doing?
13:25joncosetting JS_GC_PROFILE_NURSERY=0 will give you some info
13:26pboneokay.
13:35pbonenot sure what most of the columns mean. but teh long pauses are OUT_OF_NUSERY
13:51joncopbone: can you pastebin the output?
13:52joncothe cryptic column names have sligtly-less-cryptic names associated with them here: http://searchfox.org/mozilla-central/source/js/src/gc/Nursery.h#28
14:15pbonehttp://vpaste.net/PoGph
14:15pboneI&#39;m going to go to sleep soon.
14:16pbonethe other thing I wanted to try was modifying my patch a little and then seeing if it still happens / what changes.
14:16pbonesince (as you may recall ;-) ) at one stage it was very awesome and I boasted that it was awesome.
14:18joncosure :) let me know how it goes
14:18pbonethere aren&#39;t any hiddiously slow ones in that particular dump
14:20pbonePAGE_HIDE is often slow, but that could be normal.
14:28pbonethat must have been a red herring. I can&#39;t reproduce it.
14:28pboneI&#39;m going to sleep now.
15:05jandemjonco: very interesting, the shape thing. I think we could at least avoid this one if we added one for the getter already: http://searchfox.org/mozilla-central/rev/cbd628b085ac809bf5a536109e6288aa91cbdff0/js/src/vm/Shape.h#1585
15:06jandemnot sure how much that would help :)
15:07joncojandem: I was wondering if we can use our unique ID mechanism so we don&#39;t need to do this
15:07joncojandem: do you know how common accessor shapes are?
15:08joncojandem: it might hurt peformance in other ways
15:08jandemjonco: can we use the whole cell buffer, maybe? there&#39;s some extra code we have to do to fixup the shape tree though...
15:09joncojandem: yes sadly
15:09jandemactually that&#39;s also not great because we&#39;d trace the parent shapes etc if we&#39;re not careful
15:09jandemaccessor shapes are somewhat common, and will probably be used more..
15:10jandemwe should make sure we don&#39;t allocate native getters/setters in the nursery
15:10* jandem checks
15:12joncoI&#39;d like to try the unique ID approach because it would remove some really nasty code
15:12jandemok NewNativeFunction allocates singletons so that part should be ok
15:12joncook
15:12jandemmakes sense
15:12jandeman easy optimization would be to add the entry once if we have both a nursery getter/setter
15:13joncojandem: it does already I think
15:13jandemjonco: oh we return after we put it
15:13jandemnvm me
16:01joncojandem: oh wait my idea doesn&#39;t work at since we still need to update the pointer
16:02joncojandem: unless we put the shape in the whole cell buffer too
16:05jandemjonco: hm right. So we use the unique id for the KidsHash table?
16:05joncoyeah
16:05joncodo you think that an extra hash lookup is going to be too expensive there?
16:08jandemnot sure. It&#39;s probably okay
17:26joncosfink: thanks for filing bug 1381058, I&#39;ve got a patch for splitting the GC reason into one for each store buffer
17:26firebothttps://bugzil.la/1381058 NEW, nobody@mozilla.org Report which store buffer fills up for FULL_STORE_BUFFER minor GCs
17:27sfinkoh, that&#39;d work
18:27bhackettjandem: ping
19:13RyanVMis there a way to change how far back AWFY shows push-to-push data?
19:13RyanVMnvm, figured it out
19:21Aryxsfink: hi, there is this cgc failure on jonco&#39;s push for 1380387 and 1380967 https://treeherder.mozilla.org/logviewer.html#?job_id=114435841&repo=mozilla-inbound Shall I back them out?
19:21sfinkAryx: yes, please
19:23Aryxok, thanks
15 Jul 2017
No messages
   
Last message: 67 days and 16 hours ago