mozilla :: #jsapi

16 May 2017
02:44bz_awayjimb: still there?
02:44bz_awayjimb: JS_GetLatin1StringCharsAndLength is totally used...
02:44bz_awayjimb: in xpconnect and bindings
08:38nbp*: bbouvier:
11:13Yoricshu: djvj I have updated our gist.
11:14Yoricshu: djvj Also, nbp pointed out that we can probably make the files even smaller (and hopefully faster to parse) by introducing maximal sharing. I'll try and prototype this once I'm done with writing a first version of the independent decoder.
12:07till is nice!
12:07firebotBug 1245279 NEW, Various tests in Speedometer seem to spend quite a bit time in jS
12:08tillbbouvier: that network replay thing is quite different from webrr. It's about the network, not about local sources of differences in behavior. It also has been possible using external tools for a long time
12:09till(that's not to say that having it built in to devtools isn't nice)
12:33mrgigglesthe mozilla-inbound tree is now closed (Windows build failure - Bug 1365219)
12:33firebot NEW, Windows Build fail with 403 pulling vs2015u3 zip
14:23tcampbelljandem: ping
15:04djvjjandem: ping
15:09jandemtcampbell, djvj: pong
15:09jandempto for a few days so around intermittently
15:22djvjjandem: so I measured time-spent in self-hosted intrinsics, and came up with a prioritized list for speedometer (will come up with data for casual "browsing" as well for top sites)
15:22djvjjandem: there are many functions there that I think we are not optimizing in baseline.
15:22joncomrgiggles: can jit::FinishOffThreadBuilder gc?
15:22mrgigglesjonco: No, |void js::jit::FinishOffThreadBuilder(JSRuntime*, js::jit::IonBuilder*, js::AutoLockHelperThreadState*)| cannot GC
15:23djvjjandem: also, I wanted to ask you about the potential to have a C++-only alternative for many of the self-hosted scripts, for use in the iinterpreter and baseline.
15:23jandemdjvj: cool, what's the worst offender btw?
15:23djvjjandem: top 4: RegExpMatcher, StringSplitString, _IsConstructing, _FinishBoundFunctionInit
15:23jandemso _IsConstructing we can kill easily by checking
15:24jandemdjvj: for baseline we could maybe specialize more calls?
15:25djvjjandem: I think this is fruitful enough that I"m going to estimate time spent across all self-hosted funcs.
15:25djvjjandem: we're just blindly leeting the interpreter and baseline jump in and optimize those
15:25djvjjandem: the high-value ones should can be fast-pathed either with an optimized stub of just machine code, or an optimized stub that calls into a VMFunction directly instead of stepping into a script invocation.
15:26djvjjandem: thought _IsConstructng would require looking up a frame..?
15:26djvji.e. walking up to caller frame
15:26jandemdjvj: yes agreed we can do better for a lot of them
15:27jandemyes _IsConstructing would be easy to optimize because there's a constructing bit on the frame somewhere
15:27jandembut the new ES6 feature is similar
15:27jandemso maybe that's optimized already and we can just use that, worth benchmarking
15:27djvjjandem: jsut to be clear, this isn't checking wether the current frame is constructing
15:28djvjbut whether the first caller frame that is a script frame is a constructing call
15:28djvjso we'd have to walk up the stack until we see a non-self-hosted script frame (failing if we stop hitting baseline frames)
15:28djvjand check that to see if it's constructing
15:29jandemno it's looking at the caller's frame
15:29jandemto distinguish |new Foo()| or |Foo()| when Foo is some self-hosted function
15:29djvjjandem: ah, I see.
15:29djvjjandem: ok that makes it simpler then.
15:30djvjjandem: here's my intuition about self-hosted stuff.
15:30djvjjandem: (this is a tangent)
15:30djvjjandem: from a perf perspective, self-hosted is great for Ion perf, since we can inline across those boundaries.
15:31djvjjandem: for interp and baseline, self-hosted actually seems like it'd be a perf hit overall, since we're never going to inline, and we're choosing to invoke the interpreter or baseline jitcode which is slower than C++-compiled tight code.
15:31jandemdjvj: (it would be nice to get data on non-selfhosted array/string natives too, we fixed shift but some of them can be slow)
15:32djvjjandem: if we measure the top-time-consuming self-hosted functions, and in interp and baseline turn those into VMFunctions (maybe with some of those having optimized stubs in baseline), it should be good
15:32jandemdjvj: yeah although some operations are much faster even in Baseline (things like object allocation or scripted calls are super slow in the VM - good example is etc)
15:33bzAnything that involves prop gets is likely faster in baseline than VM, for example...
15:33djvjjandem: I can do that. GOnna post a bug with these measurements soon. I'll CC you on it. Can you comment on that with functions you'd like measured?
15:33djvjjandem: bz: good points.. calling into script, and property access probably benefits frooptimization paths.
15:33jandemdjvj: sure, offhand I'd say push/pop/shift/unshit/slice/splice
15:34djvjjandem: ok. i can remember that.
15:34djvjI guess targeting "terminal functions that don't play much with object structure" is a good bet for investigating.
15:35jandemfor instance Object.prototype.toString is slow for us atm because we have to do the @@toStringTag lookup
15:36jandemso self-hosting that would be nice and should be an improvement even in baseline
15:36jandemdjvj: that would be a good function to measure too (obj_toString), I think Angular uses it a lot
15:59tcampbellhmm.. when concatenation result is very large, we throw InternalError. Do we require consistent behavior for these exceptions? e.g. can we remove as dead-code
16:32Yoricdjvj: shu: Also, writing a pseudo-code algorithm that derives a parser from a ESTree specification is pretty simple. Certainly much simpler than from an EcmaScript BNF.
17:43jimbWho understands the initClass stuff?
17:43jimbI have a new JS-visible type whose constructor should be visible in the JS shell and for chrome code.
17:43jimbSo it's kind of like Debugger.
17:44jimbBut it seems like the new ClassSpec stuff can make the setup easier, and Debugger doesn't use that.
17:55evilpiejimb: okay I can probably help
17:57jimbevilpie: Thanks! Let me get to that branch, sec...
17:57evilpieI don't think you can conditionally define something use ClassSpec?
17:59jimbevilpie: Okay. Debugger does this:
17:59jimbevilpie: And that's nice, because the shell can just call JS_DefineDebuggerObject
17:59jimbevilpie: and we can export something to chrome code that calls JS_DefineDebuggerObject
17:59jimbbut you're saying that won't work wih ClassSpec.
18:00jimbSo I'll need to roll my own the way Debugger does.
18:00jimbevilpie: Okay, that's advice I can follow. :)
18:01evilpieyeah I am sorry, maybe moving this code do some reasonable place would be possible?
18:01evilpieClassSpec can still be useful, because it automatically supports JSXrays
18:30bzwe could do something more like dom where we have a dynamic check for whether a thing should be installed on a given global...
19:47tilldjvj: ping
19:47djvjtill: pong
19:48tilldjvj: bug 1365361 is pretty interesting
19:48firebot NEW, Baseline: Optimize intrinsics used in self-hosted functions
19:49tilldjvj: I looked at the Math.min/max uses, and it looks like we can replace almost all or all of them with simpler inline code, probably using a macro
19:49djvjtill: yeah. I was surprised by the presence of min/max so heavily in the general browsng output. what did you find interesting?
19:49tilldjvj: ToString looks like it might have overflown?
19:49till"ToString => -777334636121002 ticks (count=650917)"
19:51djvjtill: oh hmm, I ran into this before.. I don't think it's overflow.. hold on
19:52tilldjvj: at least some of these intrinsics are inlined, so for those all the hits are in the interpreter or baseline. Might be worth looking into emitting specialized bytecode for some of them, similar to what we do for a few things such as _DefineDataProperty already
19:57djvjtill: yeah, it wasn't an overflow, just some bad data cleanup in the output.
19:57tilldjvj: ah, ok
19:58djvjToString => 57046500 ticks (count=650918)
19:58djvjtill: thanks for bringing that to my attention.
20:02djvjtcampbell: ping
20:02tcampbelldjvj: pong
20:02tilldjvj: sure
20:05djvjtcampbell: so Array.push is selfhosted in js/src/builtins/Array.js
20:05djvjtcampbell: I think it's called ArrayStaticPush in the file, right? I dont' see any ICs for optimizing it.
20:05djvjtcampbell: not saying we should, but confirming that we don't special-case these calls?
20:06evilpie_FinishBoundFunctionInit seems rather high as well
20:06djvjtcampbell: in baseline IC call chains that is..
20:07djvjevilpie: yeah, makes sense as bound functions are used all over these days
20:07tcampbelldjvj: hmm. sounds right. looking
20:07djvjtcampbell: I took a quick look, but that code has changed so much these days that I'm not sure I'm missing something
20:08anbadjvj: ArrayStaticPush is the non-standard Array.push function, not to be confused with Array.prototype.push
20:09anbaevilpie: I hope bug 1365387 improves _FinishBoundFunctionInit time
20:09firebot ASSIGNED, Avoid function name atomizing to improve performance of _FinishBoundFunctionInit
20:09djvjanba: so where's Array.prototype.push defined? I searched around js/src/vm/Array.h and didn't find a C++ bultin implementation.
20:09evilpieanba: cool
20:09evilpieoh and thanks for fixing ToLength, that was important
20:10djvjanba: looked in Array.js in builtins and found ArrayStaticPush, which is the only push method in there
20:10djvjfigured it was the right one..
20:10djvjanba: nah, I think ArrayStaticPush is the right one. It's bound to "push" in js/src/jsarray.cpp using JS_SELF_HOSTED_FN
20:11evilpiedjvj: no anba is rigth
20:11evilpieArray.push != Array.prototype.push
20:11djvjI did not even know Array.push was a thing :)
20:12djvjright, so we do array_push for Array.prototype.push
20:13djvjok, I'll go instrument and measure these as well.
20:16djvjanba: thanks for the correction.
20:16anbadjvj: sure
20:16djvjwould have ended up measuring the wrong thing
20:40evilpiedjvj: till: anba: ^ a bit easier to read
20:40tillevilpie: excellent, thanks
20:41evilpiewow those intl_ functions must be _slow_
20:42ehoogeveenThe Ticks value for ToString looks like the bad old value, should be 57046500
20:44tilldjvj, evilpie, anba: I'd argue we should ignore everything with count below 10k. None of the ticks/call values are large enough to worry about single calls being an issue
20:46tillwait, the ticks/count values for Math.min/max can't be right
20:46tillor, hmm
20:47evilpieyeah that is extremely high
20:47evilpiewe definitely need to stop calling those functions
20:47evilpieseems unrealistic though?
20:47tillno, it's fairly easy to reduce them drastically
20:48tillin most cases we know we're comparing integers
20:48tillthough nothing guarantees that an if statement/ternary would actually be faster
20:49evilpiewe would stop calling into a function in interp/baseline
20:49evilpiedefinitely faster
20:49evilpie(for simple cases)
20:49tillyeah, that's probably true
20:49evilpiecould even add JSOP_MATH_MAX ;)
20:49tillok, let me whip up a quick patch
20:50tillyeah, I suggested that above
20:50tillbut for now we can just use a stupid macro
20:54tillit seems exceedingly unlikely that math_min would be more than 5x as slow as function_apply ...
20:57anbaWas Ion disabled when these performance numbers were collected?
21:07djvjanba: no
21:07djvjanba: I want to identify how much we're invoking them in reality
21:08djvjanba: if Ion is optimizing them out, then we're already addressing them in those cases.
21:08evilpieprobably not much ;)
21:09evilpieactually not true with all those new ember/react stuff that keeps on calling the same functions
21:09anbadjvj: I would have expected functions like std_Math_min were already inlined in js/src/jit/MCallOptimize.cpp
21:10evilpiesadly we don't inline when we have bad type info
21:10evilpiemaybe we should fix that
21:10evilpieor maybe even self-host them
21:11djvjtill: yeah, the min/max numbers are weird to me too
21:11djvjtill: it's super strange.
21:12tilldjvj: it is!
21:12tilldjvj: but they're pretty easy to replace in most cases, at least
21:13djvjtill: I mena, the min average is about 152microseconds per call, according to those stats
21:13djvjI should double check those and confirm, but man..
21:13djvjmaybe they're getting called with lots of numbers?
21:16djvjtill: and 77 microseconds for an average call to max() ?
21:16tilldjvj: yeah, that, too is pretty weird
21:17tilldjvj: we're not using the min/max intrinsics with more than 2 args
21:18djvjI'm going to make a note in the bug to go over these again
21:18djvjI think I need to modify my python script to throw out outliers
21:19djvjI might be hittinig cases where there's a context switch inside a call, and that's causing the recorded times to be wildly off
21:23tillah, that would make sense
21:24tilldjvj: though it'd be pretty weird if min and max were both affected purely by chance. The other similarly costly function, i.e. the intl stuff, seem legit
21:25djvjtill: I know, especially since the counts are so low.
21:25djvjbut it might be fluke?
21:26djvjI'm going to update the scripts to do a histogram of the numbers for each function, plot them, and go through them to look for anomalies
21:26djvjlook at me ma, I'm a data scientist now!
21:26djvjnext step: make the data big
21:32anbaevilpie: Are the "in" operator optimizations stable enough, so I can use the "in" operator instead of IsPackedArray in Array.prototype.indexOf/lastIndexOf ? (bug 1364363)
21:32firebot NEW, Incorrect IsPackedArray optimizations in Array.prototype.indexOf and Array.prototype.lastIndexOf
21:36evilpieanba: I have to check if we can totally eliminate it in ion
21:36anbaevilpie: ok
21:49tilldjvj, jandem-away, evilpie: this should behave better in the interpreter and baseline:
21:49tillquestion is if it'd regress ion
22:08jimbWhat are the rules for reporting OOMs these days? Is it still just: return failure with no exception set?
22:08jimbI thought there were plans to make it more specific
22:13firebotBug 1365042 NEW, More than test262/language/{expressions,statements}/async-generators need to be skipped if release_o
22:14leobalterI've got to go, butI'll catch up with anything necessary tomorrow in the morning
22:14leobalteranba ^^ Thanks!
22:14anbaleobalter: np
22:14anbaleobalter: and thanks for fixing the flags -> features issue in test262 :-)
22:15leobalternot a problem as well :)
23:14shuleobalter: great, thanks for the patches
17 May 2017
No messages
Last message: 128 days and 9 hours ago