mozilla :: #jsapi

21 Feb 2017
01:55araihmm, BytecodeParser returns pre-condition for the given PC, but I need post-condition... :/
09:46h4writernot sure which locale my computer started to use, but it is macabre
09:46h4writer> Die Feb 21 2017
10:09bbouvierjandem: thanks for the review! just to be clear, are you suggesting just moving the call of updateMallocBytes to the callers, or calling the GCRuntime's function also named updateMallocBytes?
10:10jandembbouvier: moving it to the callers (and for ExecutableAllocator use the actual allocation size not the pool size)
10:11jandem(by actual allocation size i mean the size we need for the JitCode)
10:11bbouvierjandem: okay, will do
10:11jandembbouvier: cool
10:28nbpjandem: bbouvier: I've one concern about reporting ExecutableAllocator allocations to the GC, which is that we now risk to GC IonCode which just got compiled & linked.
10:28bbouviernbp: yes, that was my worry too that it might affect performance
10:29bbouviernbp: that being said, if the JIT allocation causes a GC, the GC is not far anyways, and the JIT code might get discarded soon anyways
10:29nbpbbouvier: not exactly, one of the GC trigger is if we do any large allocation.
10:30nbpbbouvier: and function in which we inline a lot can be quite large.
10:30jandemthis is just thee mallc count
10:31nbpjandem: and the GC trigger is as well just the malloc count
10:31jandemjonco: what's the typical max malloc bytes value for a Zone?
10:32nbpthe largest ion compiled function on octane was a few MB, IIRC.
10:34nbpjonco: also, what is it on mobile?
10:34joncobbouvier: it's not that one
10:35bbouvierjonco: oh really? it seemed to converge to that one value, from the JS_NewContext calls in xpcjsruntime
10:35bbouvierand this value is passed to Zone::setGCMaxMallocBytes
10:36joncoit is? I thought that was the GC heap max size
10:36nbpbbouvier: we have about:config settings to change the max-malloc trigger.
10:37bbouvierlet me try to confirm with a debugger...
10:38nbpjavascript.options.mem.gc_allocation_threshold_mb ?
10:38nbpjonco: ^ I see 3 on android, and 30 on desktop.
10:39joncobbouvier: you're right, the same value is used for both settings when creating the GCRuntime
10:40nbpjandem: I don't recall if I saw 3 MB or larger, but it is definitely closer to what we reached on octane, and ARM assembly might also be more verbose.
10:40bbouvierjonco: it's actually 90% of that value :)
10:40joncobbouvier: right, yeah
10:40jandemok yeah that is pretty small and we should be careful
10:41bbouvierjonco: does it make sense that this never gets called at startup?
10:41smvvany idea why this ion invalidation happens? is it related to singleton objects?
10:42bbouvierjonco: (asking because this is called indirectly by JS_NewContext too)
10:42smvvi'm running a debug shell with -D --no-threads --ion-inlining off --ion-scalar-replacement off
10:42nbpsmvv: Can you run it under rr, and backtrack to the caller of the invalidation?
10:42nbpsmvv: or the trigger of the invalidation
10:43joncobbouvier: it gets called from GCRuntime::init, or is that not what you mean?
10:43nbpsmvv: IONFLAGS=bailouts should also help to highlight what happened before.
10:44smvvnbp: it's not a bailout. I have bailouts and bl-bails set
10:44smvvnbp: i'm using export IONFLAGS=bl-ic,osi,bl-bails,bailouts,scripts
10:44bbouvierjonco: i see it should be called from GCRuntime::init, but using a debugger and adding a breakpoint on Zone::setGCMaxMallocBytes, I don't see it being called *here* at startup
10:45nbpsmvv: what you see is the polymorphism which invalidate the code as new types have been observed, which invalidate the constraint produced during the compilation.
10:45bbouvierjandem: another option is to update the malloc bytes count only for wasm code, which are pretty big ; and the wasm code wouldn't get discarded right away, since the WasmInstanceObject holding it is still alive?
10:45smvvis it because these types are singleton objects?
10:45smvvnbp: the ones that happen after the first loop
10:45nbpsmvv: this is because the code got compiled assuming only one type, and you provide a second one.
10:46nbpsmvv: singleton objects would appear as different types, yes.
10:46joncobbouvier, jandem: we could also consider have a separate counter for complied code
10:46smvvnbp: what is the advantage of singleton objects? I'm aware of objects that are allocated in the tenured heap, but not what singletons are used for
10:47bbouvierjonco, jandem: oh right, i think it was one of the original ideas
10:47bbouvierjonco, jandem: yeah, let me do that, it will probably be cleaner.
10:47bbouvierjonco: would that counter live in the Zone too? (or JitZone?)
10:47nbpsmvv: That's a long debate, but mainly for object created once, and where we would only have a single instance, such as "JQuery = "
10:48jandemyeah that makes sense to me. We could also use that in shouldPreserveJitCode later
10:48joncobbouvier: it's set when the zone is created from the constructor (it does seem to get set twice if GCRuntime::setParameter is called with JSGC_MAX_MALLOC_BYTES which seems wrong though)
10:49joncobbouvier: Zone makes sense I think with the other GC counters
10:50smvvnbp: what is currently the difference between a singleton and an object allocated in the tenured heap? Or do you know where I can find more information about the difference?
10:51nbpjonco: I don't think we have time for that right now, but I found that funny that in our code base, we have values for this settings which goes from: 1 (b2g), 3 (android), 6 (Shell testing), 30 (firefox), 32 (servo)
10:51smvvnbp: or is it only semantically (object created once should be a singleton)?
10:51bbouvierjonco: okay, any idea for an initial code size limit?
10:51bbouvierjonco: or i guess i can just use awfy and wasm to find a value that at least does not regress benchmarks
10:52joncobbouvier, jandem: we can also make this a proper count of currently allocated memory (unlike the malloc count which just counts down on allocation and doesn't take account of free) because we do know the size on free
10:52joncobbouvier: that sounds like a good plan :)
10:53nbpbbouvier: if we preserve jit code when we pull this trigger, then we can set it quite low as well.
10:53bbouviernbp: well, i think the idea is to trigger a GC is the limit is to be reached
10:53bbouvierjonco: so we'd also update this new counter when we *release* executable memory, to get a precise count?
10:53bbouvierthat sounds nice
10:53joncobbouvier: yes
10:54bbouvierjonco: cool, it sounds more precise than just the malloc counter bytes indeed
10:54joncobbouvier: we'd like to do this for malloc too but we don't always know the size (or that was true last time someone looked which was a while ago)
10:54joncobbouvier: yes
10:55nbpbbouvier: jonco: don't we risk frequent-double GC, if we have a simple threshold checking against the sum value?
10:56jonconbp: why would you get a double GC
10:56nbpsmvv: I do not recall exactly the rules for singleton, they are a few hard-coded one, but I think this is when we compile and only see a single object that we promote it to become a singleton
10:57nbpjonco: the first freeing a tiny bit of memory putting us under the threshold, and the second being done after going over the threshold again after some IonCode allocation.
10:59nbpsmvv: based on your test case I would not expect singletons, but a single TypedObject from the allocation site coming from the "first" function.
10:59jonconbp: right that is true for the GC heap we increase the threshold based on the remaining data allocated (e.g. we double it, with some exceptions)
10:59jonconbp: maybe we should do that, or maybe an allocation count would be simpler
12:13evilpieh4writer: thank you
12:13evilpiedo you think you will have a chance to add ARES-6 to awfy?
12:15jandemyes please
12:18jandemh4writer: it would be nice to have dromaeo back too, maybe not on every check in but a few times a day maybe
12:18jandemnot super urgent and I know the browser benchmarks are more annoying to run :/
12:20evilpieARES-6 has a cli mode even
12:22jandemevilpie: how much time does it take? wondering if we could add it to the assorted suite
12:23evilpienot even a minute
12:24evilpiethey do have this line that I am not sure how to best port to our shell
12:24evilpieI changed this to g = newGlobal() g.eval() but afterwards 90% of the time seems to be spend in promise resolution CCWs ..
12:39h4writerevilpie: jandem I would definitely prefer a shell benchmark. Can we replace it with something logical?
12:40jandemif it involves promises, a browser benchmark might be more representative unfortunately
12:40h4writerjandem: I see
12:40jandemmaybe we can compare them
12:40jandemand if the difference is small the shell version will do
12:41h4writerevilpie ^^ is the score in-browser / in-shell similar?
12:41evilpiecan't really compare them
12:41evilpiein browser even triggers slow script
12:41h4writerjandem: I never disabled dromeao :O
12:41h4writerjandem: this must be a running issue
12:42jandemh4writer: ah
12:42jandemevilpie: ugh
12:42jandemnot sure what we should do then
12:43h4writerjandem, evilpie: I'll try to get them fixed
12:43h4writer*and added
12:44evilpiesix-speed ( isn't awesome, but v8 is tracking that internally as well
13:04evilpieWe should have some policy about implementing new features at least require filing a bug about optimizing in the JITs
13:04h4writerI agree on that
13:06nbprest arguments: 1558x faster :cough: dead code elimination :cough:
13:07nbpI would prefer if we did not had to care about new features in the JIT.
13:08nbpI mean, I would prefer if this concern did not even exists, because it would be a seamless process.
13:09nbpI have a solution for that!
13:09evilpiethat would be nice, but realistically any less than trivial feature is going to need to special support
13:09jandemevilpie: arai already inlined IntrinsicGetNextMapEntryForIterator and IntrinsicGetNextSetEntryForIterator in Ion, pretty nice
13:10nbpevilpie: let me disagree, and maybe later proove you wrong.
13:11bbouvier"just wait for all our users to stop using firefox because es6 features are too slow"?
13:13h4writerbbouvier: not sure if this specific to "es6 features"
13:14nbpInteresting, Webkit does some "type-inference" on the keys of Map and Set.
13:17nbp^ see map-set-lookup , which highlight the dead-code elimination results of the map.has & set.has functions.
13:53evilpiejandem++ DOMProxyUnshadowed finally gets rid of CCS2Properties like width, top on gdocs!
13:55jandemevilpie: nice
13:56jandemevilpie: do you see a lot of generic proxy stubs?
13:57evilpie22 vs 45 unshadowed
13:57jandemas i mentioned before, the unshadowed code in Ion is broken
13:58jandemso we might see some benchmark wins when we port that. dunno
13:58evilpietwo that seems to set an expando on HTMLDocument
13:58evilpiethe rest is just "Proxy" so probably a CCW
13:59evilpiewould be nice to give a name to handlers
13:59jandemyeah because they are virtual classes already we could just add a virtual method for that
14:00h4writernaveed: ping
14:01evilpiejandem: fixing bug 1336580 would still be a huge win
14:01firebot NEW, AddSlot ICs fail because of unperformed new script analysis
14:02jandemyes it's also on my list, hopefully in a week or two
14:03jandemevilpie: some of the generic proxy ones could be WindowProxy maybe? or are they reported differently
14:03decodernow what is evalInCooperativeThread ? :D did we add that recently?
14:03jandemdecoder: bhackett did
14:03evilpieI think WindowProxy has its own JSClass
14:04decoderjandem: so I guess I should try using it to run code?
14:04jandemdecoder: yes please
14:04decoderjandem: does it behave like evalInWorker when it comes to global? or does it share the global?
14:05jandemdecoder: it behaves like evalInWorker afaik
14:05jandemdecoder: actually I'm not sure, let me check
14:06jandemdecoder: yeah different global looks like
14:06decoderjandem: okay, then i will try to test it like evalInWorker
14:07jandemdecoder: cool
15:15h4writerjandem: I remember again. I disabled dromeao since it was somehow behaving badly on one of the browsers.
15:15h4writerjandem: I'm going to enable and see if it is still the case
15:16jandemh4writer: great, thanks
15:24tcampbellCan a Handle<T> be converted to a Handle<U> using an as<U> conversion?
15:26joncotcampbell: yes for JSObject*
15:30tcampbelljonco: Handle<T*> x = &jsobj->as<T>(); complains. What am I missing?
15:30jandemtcampbell: do .as instead of ->as
15:32tcampbellit&#39;s a slow adjustment back from a long weekend
15:32h4writertcampbell: ping
15:33tcampbellh4writer: pong
15:53decoderjonco: turned out the other assert was bug 1340532
15:53firebotBug is not accessible
15:54joncodecoder: ok great
16:05h4writerevilpie: do you know if ares has subscores?
16:07evilpieh4writer: they measure different stuff like avg or first run, but there are two main benchmarks Air and Basic I think
16:07h4writerevilpie: ah and no extra scores. I see
16:07h4writerevilpie: I thought it would show different scores for different ES6 features
16:08evilpieMaybe you are thinking of six-speed?
16:09h4writerevilpie: probably
16:09h4writerevilpie: which one would make the most sense? Six-speed or ares-6 for es6 features?
16:10evilpieSix speed isn&#39;t very good, but shows single features we can optimize
16:11h4writerevilpie: I see
16:12evilpieAre we resource constrained?
16:13h4writerevilpie: We only have one machine and having it take 2h to complete, means that the regression range is quite long :S
16:17jandemh4writer: also the windows box? dromaeo is pretty slow i think so we could run that less frequently
16:17h4writerjandem: I&#39;m currently looking to use the win8 machine because of that
17:03a_v_ihello everyone,
17:03a_v_iim interested in systems programming and compilers.... looking for suitable bugs to fix .... great if you point me out to some mentored bugs that are still in need of work..
17:26h4writerarai: ping
17:37* Ms2ger looks for a timezone-appropriate GC person
17:37Ms2gerjonco, still around?
17:38joncoMs2ger: hi
17:39Ms2gerjonco, is JS_AddExtraGCRootsTracer known to have issues with nursery-allocated objects?
17:39joncoMs2ger: yes, it doesn&#39;t get called during nursery collection
17:39joncothe tracer that is
17:39joncoMs2ger: what problem are you having?
17:40joncoMs2ger: pointers into the nursery are usually handled by having a barrier add a store buffer entry for that location
17:41Ms2gerI&#39;ve narrowed down my problem to &quot;shit crashes&quot; so far :)
17:41jonconice :)
17:41joncothis is the usual symptom
17:41Ms2gerAre you around tomorrow?
17:42Ms2gerOkay, I&#39;ll try picking your brain then :)
18:51* Waldo stabs his review queue again
20:50Waldomrgiggles: pun
20:50mrgigglesWaldo: I try wearing a pair of tight jeans, but I can never pull it off.
20:50evilpieWaldo: what&#39;s an anagram for milk?
20:51Waldoevilpie: 98% sure there isn&#39;t one
20:52evilpieokay good, maybe this is a troll
20:53evilpiefrom this HN thread btw
20:59Waldo|ack -i &#39;^[milk][milk][milk][mlik]$&#39; /usr/share/dict/words| only gives back milk as fitting that query, at least
21:05shuWaldo: did you see bug 1341061?
21:06shuWaldo: argh
21:06firebot NEW, post-increment operator inside &quot;with&quot; ends up checking @@unscopables twice
21:06Waldoshu: I saw whatever bugmail issued through about yesterday evening in it, yes
21:06Waldonot surprised our bind* stuff isn&#39;t quite to spec
21:06shuWaldo: well, for gets, yeah
21:08shuWaldo: but our BIND stuff isn&#39;t that far off from References
21:08Waldomost certainly
21:09* shu wonders how to fix this without breaking all the JITs
22:09mrgigglesthe mozilla-inbound tree is now closed (bustage from bug 1284897 )
22:09firebot NEW, 64 bit Flash Player has storage permissions issues
22:29mrgigglesthe mozilla-inbound tree is now open
22:31araih4writer: pong
22:32shutill: i am really swamped, i hope to have time to review streams this week, sorry
23:04mrgigglesthe mozilla-inbound tree is now closed (bustage from bug 1341023 )
23:04firebot ASSIGNED, CreateJSStackObject is not taking the encoding of the module name into account
23:33grenstjohn-galt you cunt
23:33grenstspineless dick sucker
23:33grenstjohn-galt show yourself bitch
23:33* Waldo flexes
23:34John-GaltSorry about that. Alas, it looks like he intends to spam every channel I&#39;m in.
23:35Waldopresumably there&#39;s some actual reason he was complaining, but I&#39;m not cutting any slack when he opens up the criticism that way
23:35Waldo(whether or not the reason is or is not ultimately justified)
23:36John-GaltHe&#39;s spamming here because I banned him from other channels for doing the same there.
21 Feb 2017
Last message: 58 seconds ago