mozilla :: #jsapi

13 Jul 2017
00:00bzzhitingz: well, are we ending up with an MLoadFixedSlot per IonBuilder::inlineUnsafeGetReservedSlot ?
00:00bzOr do we still have an MCall?
00:01bzIs the slot index constant?
00:01bzin hte JS?
00:01bzer the JS
00:01zhitingzyes
00:01bzok
00:01* bz is looking over things that might prevent inlining there
00:01zhitingzthat's constant323
00:01zhitingzin the above example
00:02bzok
00:03mrgigglesthe mozilla-inbound tree is now open
00:17Bakkotquestion: I have a test which fails when run with jstests.py but not when run without it. how do I get more useful information than just "it failed"?
00:19bz_awayBakkot: -o
00:20bz_awayAnd perhaps -s so you know what it's really running
00:23Bakkot-o didn't give anything helpful, but -s is nice
00:24Bakkotthanks! (my issue: turns out you need to include a call to `reportCompare` in the test, I guess. -o doesn't seem to complain when you don't, but you still get a failure.)
00:27erahmIs there a reason we don't have a version of js::DuplicateString(JSContext* cx, const char* s) that takes a length?
12:40smaugjonco: do you recall why InvokeInterruptCallback calls gcIfRequested?
12:49smauglooks like some really old code
12:57djvjjonco: ping
12:58djvjjonco: was looking at this yesterday with tcampbell. It seems we're discarding baseline jitcode even during incremental GCs? Is this desired behaviour?
12:58djvjhttp://searchfox.org/mozilla-central/source/js/src/jsgc.cpp#4042
12:59djvjjonco: ^-- we have DiscardJITCodeForIncrementalGC() being called in beginMarkPhase(), which calls Zone->discardJitCode() with a default second argument of (discardBaselineCode=true).
13:01djvjseems expensive to discard baseline jitcode every time we start an incremental gc. Is there a reason it's being done this way or is it just that we're forgetting to pass /* disacrdBaselineCode = */ false in the call to Zone->discardJitCode()
13:15jandemI added that argument but it's for something else, either we should discard everything or nothing
13:15jandemwe have some heuristics that set the preserving-code flag
13:15jandembut there's a lot of room for improvement
13:28verpeteren12
14:36tcampbelldjvj: regarding our discussion yesterday. I observe 20% of BBs are culled by restartLoops for overall spedometer, and 90% in the Netflix case
14:37tcampbell(which seems to be about 200MB wasted on LifoAlloc in Netflix case)
14:43joncosmaug: we if we hit one of our thresholds while allocating on another thread we trigger an interrupt to start the GC ont he main thread
14:47smaugjonco: but don't we end up triggering gc slice also if js execution takes lots of time and there is pending igc ongoing?
14:49joncosmaug: we do that too
14:50smaugjonco: and that is like the worst time to run a slice when running benchmarks
14:50smaugit seems to happen occasionally when running speedometer
14:50joncosmaug: yes but it helps greatly with splay latency
14:51joncoI'm not sure what the answer is
14:52smaugjonco: which splay latency?
14:52smauger, should it be 'what'
14:53joncosmaug: the octane benchmark
14:53smaugI wonder how to run that
14:54smaugIIRC we run only Kraken as part of talos
14:54joncosmaug: https://chromium.github.io/octane/
14:54joncoit's on AWFY
14:54smaugsure, but one can't run awfy locally easily
14:55smaugI'll try that chromium page
15:03mrgigglesthe mozilla-inbound tree is now closed (Bug 1380381 - build symbols missing on macOS/OS X )
15:03firebothttps://bugzil.la/1380381 NEW, nobody@mozilla.org build symbols missing on macOS/OS X, unhelpful crash signatures like [@ XUL + 0xddb7c]
15:09lukejonco: is there any easy way for a random thread to make a low-priority request for a JSRuntime to do a full GC?
15:10lukeother than dispatching a runnable to do so on the JSRuntime's thread?
15:15joncoluke: that would be the obvious way to do it
15:16joncoI can't think of anything better right now
15:17lukejonco: yeah, makes sense; i was just wondering if there was already some existing off-thread "poke"
15:18joncoluke: there is if you're a thread from that runtime that has a JSContext
15:18joncowas that what you meant?
15:18joncoif so GCRuntime::requestMajorGC() will work
15:19joncoor in fact triggerGC() would be better
15:20lukejonco: do you have to be on that JSContext's active thread?
15:21lukejonco: or just have a pointer to the JSContext
15:21joncoluke: you have to be on the thread
15:21joncothere's no way to trigger it from a completely unrelated thread
15:23lukejonco: ok, makes sense, thanks
17:03djvjjandem: why can't we preserve baseline jitcode on incremental gc? are there potential nursery pointers we can't update?
17:04jandemdjvj: we already do preserve jit code, there's a preserve-code flag that we set sometimes
17:04jandemso there's no hard reason, just a matter of heuristics
17:05djvjjandem: not during begin-mark phase of inc-gc, it seems.
17:05djvjah, ok
17:06djvjjandem: I'll make a bug for this and test keeping baseline jitcode around.
17:06jandemdjvj: there's a jit code quota now we have to be a bit careful
17:06jandemif we change heuristics i'd prefer treating ion/baseline the same way
17:06djvjjandem: how does the quota work?
17:06jandemjust a per-process limit
17:08djvjjandem: I'm not sure that makes sense. baseline jitcode is more reliable than ion (e.g. won't invalidate) and should be treated as "more valuable". Anyway, I'll see if it does anything to mess with those knobs.
17:09jandemdjvj: on the other hand Ion code is much slower to compile and there's usually much less Ion code
17:12djvjjandem: true, but the sunk cost of compilation doesn't matter going forward. When we discard a piece of baseline jicode, the probability that we'll soon re-generate it is high. Less so for ion jitcode (and we'll pay more to generate it when we do).
17:12jandemdjvj: well let's just treat them the same way, as we do now, until we have evidence to change that
17:12djvjjandem: ideally, we could use some LRU mechanism to keep the most recently invoked baseline scripts and discard others.
17:13jandemyeah we could do with better heuristics here
18:02jorendorffI haven't pushed anything since the work week, and now:
18:02jorendorffWarning: the ED25519 host key for 'hg.mozilla.org' differs from the key for the IP address '63.245.215.102'
18:02jorendorffdid it change?
18:07nbpjorendorff: the closest google recall from dev.platform comes from March 2016.
18:07jorendorffbizarre, what the heck
18:08jorendorffI retried and it mysteriously went away 8-|
18:08nbpjorendorff: maybe a restored ~/.ssh/known_hosts made its way to your system?
18:09nbpA double bit flip network communication issue?
18:10sfinkyour mitm hacker slipped up, but he fixed the problem now
18:11jorendorffah good
18:12jorendorffthe best I could come up with was, "glitches in the matrix"
18:12nbpoh, have you spotted a cat doing the same thing as it was 5 seconds ago, such as sleeping?
18:32fitzgensfink: ah! I remember why I pung yesterday
18:32sfinklook, a squirrel!
18:32fitzgensfink: was wnodering how hard it would be to write an analysis to check for proper JS_PUBLIC_API/JS_FRIEND_API usage
18:33sfinkI dunno, step 1 sounds hard
18:33sfinkwhich is to explain to me what proper usage is
18:33fitzgenI think the answer is "all non-inline functions / static variables in a public header need it"
18:33fitzgenunclear when classes/structs need it
18:34fitzgenI think anything that needs to be linked needs it
18:34sfinkwhen do you want PUBLIC vs FRIEND?
18:34fitzgensfink: they are the same, its just about our "stability guarantees" for the API, which lets not get into ;)
18:34sfinkoh, ok
18:35ptomato-Msfink: fitzgen: I'm interested in the answers to these questions as well :-)
18:35fitzgenptomato-M: yes, I was thinking of your recent patches :)
18:35smaugwe run minorGC all the time when running speedometer. I wonder if there is something to optimize there
18:35smaugeither run less often or make it run faster
18:37sfinkso... could you do a compile, dump out all symbols, then make a dummy .cpp file that refers to all of those symbols, compile that while #including all public headers, and anything it complains about is considered to be a private symbol. Then check the linkage on all public symbols?
18:37sfink(massive handwaving here)
18:37sfinksmaug: do you know the reason on those minor GCs? (Or point me to a profile, and I think I can extract them)
18:38smaugsfink: I think alloc, but let me create profile
18:39sfinkugh, this random profile I have lying about doesn't have the full minor gc info. I did implement that, didn't I?
18:39fitzgensfink: I was imagining maybe a clang plugin that walked every definition in js/public/* and js/src/js[friend]api.h; I wonder if making obj files and running readelf/nm would be easier
18:39sfinkI need to learn how to do clang plugins
18:39sfinkthough I guess I might have that data already in my gcc plugin
18:43smaughrm, I don't get symbols
18:43smaugshared from Nightly
18:44sfinkosx?
18:44sfinkthough for this purpose, symbols don't matter
18:47smaugsfink: https://perfht.ml/2vhxjmh
18:49sfinkugh. We don't seem to store the reason there. (It does have the detailed timings, at least.)
18:50smaugI'm not familiar with minorGC reasons
18:51smaugI thought minorGC would run on alloc if needed, and also before majorGC
18:51sfinkthere aren't many. FULL_STORE_BUFFER is another one that is fairly common.
18:53sfinksmaug: ok, since the profile doesn't have that data, can you do a run with env var JS_GC_PROFILE_NURSERY=100 ?
18:53smaugsure
18:53smaugwhat does that do?
18:53sfinkprints out some basic timing data for any minor GC that takes more than 100 microseconds
18:53sfinkyou can drop that to 1 or something if you want
18:54sfinkmaybe set JS_GC_REPORT_TENURING to, uh, 100 too
18:54sfink(that's the minimum number of objects per ObjectGroup to report)
18:54sfink"After a minor GC, report any ObjectGroups with at least N instances tenured."
18:55smaugsfink: https://pastebin.mozilla.org/9027028
18:55smaugthat was without JS_GC_REPORT_TENURING
18:55sfinkyeah, that's fine, they're independent
18:56sfinkthat's a surprisingly even distribution across reasons
18:57sfinkI haven't stared much at these, so I dont' have a great sense for what to expect
18:57sfinkbut given the number of OUT_OF_NURSERY ones, I wonder if it'd be better to have a larger nursery for this
18:58sfinkthere are 2 brutally slow minor GCs in there
19:01sfink5.876 ms to resize seems excessive
19:04smaugsfink: I assume we don't dynamically increase nursery size
19:04smaugif we get out_of_nursery often
19:04sfinkwe do, up to some limit
19:04sfinkhm, you didn't have an output earlier that started with "MinorGC: Reason PRate Size"?
19:05smaugthat was a question?
19:05sfinkthat Size is the size of the nursery in 1MB chunks
19:05sfinkyes, it looks like that should have appeared in your output
19:06sfinkit's not a big deal, it just explains the fields there
19:06smaugwhat does FULL_STORE_BUFFER mean?
19:06sfinkit looks like we cap at 16MB
19:07smaugah, we start from DefaultNurseryBytes and cap at 16MB?
19:08sfinkI'm trying to figure that out. DefaultNurseryBytes is 16 chunks already
19:08smaugdefault is 16777216 or 4194304
19:08smaugdepending on JS_GC_SMALL_CHUNK_SIZE
19:08sfinkit looks like Nursery::init starts out at 1 chunk
19:09sfink(1MB or 4KB for JS_GC_SMALL_CHUNK_SIZE)
19:10sfinkI don't see any pref to control maxNurseryBytes :(
19:11sfinkoh, I see your email, I guess it did have that header line
19:22jimbSince Waldo's out, who's the next best C++ authority?
19:23sfinkfroydnj|pto is good, I would expect botond to be good
19:23jimbbotond naturally
19:24jimbI seriously wonder how much effect the admonition not to go on PTO after the all-hands had
19:24jimbI can think of three people off the top of my head
19:24jimbincluding managers
19:25evilpiesfink|afk: I have some experience with the clang plugin
19:25sfink|afksmaug: I'll be offline for a bit, but I filed bug 1380768 bug 1380770 bug 1380773
19:25firebothttps://bugzil.la/1380768 NEW Add preference for setting maxNurseryBytes
19:26firebothttps://bugzil.la/1380770 NEW Add minor GC reason to MinorGC marker
19:26firebothttps://bugzil.la/1380773 NEW Consider adding large tenured promotion counts to MinorGC marker
19:26sfink|afkI haven't really come to any real conclusions about the speedometer minor GC stuff
19:26sfink|afkis there a bug for that yet?
19:27smaugnot that I know
19:27sfink|afkI think it'd be useful
19:29smaugI'll file
20:17ptomato-Mfitzgen: speaking of JS_PUBLIC_API and whether it belongs on classes / structs - I ran into another one. JS::ConstUTF8CharsZ::validate() doesn't link when you try to create a JS::ConstUTF8CharsZ in a debug build
20:17ptomato-Mfitzgen: but does JS_PUBLIC_API belong on the class itself, or only on the non-inline method (validate())?
20:17fitzgenptomato-M: It isn't clear to me...
20:31jdmjimb: I thought the admonition was not to go before?
20:32jimbjdm: or after
20:36jandemno after was fine, it had the july 4th holiday so it was a good opportunity
20:37jandemor maybe there was another one
22:21ptomato-Mfitzgen: should I make a new bug for the ConstUTF8CharsZ patch, or attach the patch to the same one?
22:29fitzgenptomato-M: new bugs are usually easier
23:28pbonemorning.
14 Jul 2017
No messages
   
Last message: 72 days and 6 hours ago