mozilla :: #jsapi

7 Sep 2017
09:48mrgigglesthe mozilla-inbound tree is now closed (windows bustage from bug 1396368 )
09:48firebot REOPENED, Crash in nsPIDOMWindowInner::UpdateWebSocketCount
10:21mrgigglesthe mozilla-inbound tree is now open
13:13nbpstandups: Closing 6 digits bugs can still help performance (Bug 966743)
13:13standupsOk, submitted #50636 for
13:13firebot ASSIGNED, IonMonkey: Modify MArrayPush to accept multiple arguments
13:18tcampbellstandups: Try privatizing indirect eval when using NSVOs (Bug 1396050)
13:18standupsOk, submitted #50637 for
13:18firebot ASSIGNED, Support IndirectEval with NonSyntacticVariablesScope
13:25Yoricstandups: Bug 1361102 - nsUpdateService.js does main thread IO writes for the update xml files during the first startup after an update, and whenever an update is downloaded - I/O expertise
13:25standupsOk, submitted #50638 for
13:25firebot ASSIGNED, nsUpdateService.js does main thread IO writes for the update xml files during the first startup afte
13:25Yoricstandups: Bug 1397717 - Move *::dump() to GenericPrinter - v1
13:25standupsOk, submitted #50639 for
13:25firebot NEW, Move *::dump() to GenericPrinter
13:33Yoricnbp: Done. Your turn :)
13:41smaugjonco: FYI, looks like CC time (from start of first slice to end of last one) is currently (95th percentile) 670ms. But this is noisy, and it was 1200ms still couple of weeks ago. Median time is 10ms, which is probably just one slice.
13:42smaugthat was child process. Parent process has lower numbers
13:43joncosmaug: ok that's lower than I thought
13:43smaugjonco: if I look at the measurement dashboard, I do see 0.03% taking > 10s
13:44joncough, that sounds bad that's pretty rare though
13:47smaugrandom note, from one can easily see what is the max idle time (there is an explicit limit in the spec).
13:48joncoheh, nice
13:48joncosmaug: I was going to ask you - why is that the limit?
13:48joncoand is it possible to increase it?
13:48smaugjonco: the limit comes from some UX research
13:48smaughow input events should be handled
13:48joncoright ok
13:50smaugmy guess for very slow GCs or CCs is that one is using slow machine with old style harddrive and the system is paging a lot. GC and especially CC needs to touch memory allover
13:51tcampbellI've gotten very long CCs in last week on fast hardware too
13:51tcampbell*last month
13:51joncosmaug: yes this could easily cause us to go way over our budget
13:52joncoI wonder if we could check do the timeout with a timer rather than checking it ever N operations
13:53tcampbell(oh, oops, my example profile is now two months old, so probably invalid)
13:58nbpYoric: no, your turn ;)
13:59* tcampbell wonders if we are at the point where some bugs are older than the people fixing them
14:02nbptcampbell: Do we have 15 years old contributor, and bugs from 2002 ?
14:03tcampbellI think we do
14:04nbpI guess we should keep these bugs around, just for the fun of giving them to our youngest contributors.
14:06bkellywas tbourvon an intern? I tried replying to him on dev-platform and his email bounced
14:06bkellyabout a js engine thing
14:28jandembkelly: don't think so
14:28bkellyhmm, ok
14:29bkellyjandem: I guess I'm wondering if he have any bugs filed to investigate using the clang overflow builtins:
14:29jandemthe jits emit their own machine code for overflow checking
14:30jandemit's like ADD; JO
14:31jandembkelly: we do have this:
14:32nbpbkelly: yes.
14:32jandemwould be nice to move this into MFBT
14:32nbpbkelly: he was an intern and he left a week ago.
14:32bkellyI guess I just wanted to check to see if this is something we should get filed so it doesn't get lost... since it had such a big perf win on chrome
14:32nbpbkelly: but not on the jsengine, he did some static analysis.
14:33nbpbkelly: Sylvestre might know better, and might have a contact information.
14:33bkellynbp: well, I don't need to talk to him directly... he was mainly the one who responded in the dev-platform thread
14:44nbpbkelly: the leak you are seeing with the JSBC, it might be a JSBC bug that I am trying to isolate and understand.
14:45firebotBug 1395233 ASSIGNED, JSBC: 3% Heap Unclassified summary windows10-64
14:47bkellynbp: ok, its just a theory that the leak I saw was jsbc
14:47bkellynbp: I haven't seen it again since disabling jsbc
14:48nbpbkelly: how did you noticed it?
14:48bkellynbp: excessive memory usage and using about:memory...
14:48bkellynbp: it seemed like mostly ad related things leaked in news pags
14:49nbpbkelly: heap-unclassified?
14:49bkellynbp: not specifically... there were a bunch of leaked windows
14:50bkellynbp: I attached the full memory report to bug 1397062
14:50firebot NEW, many ghost windows in nightly 57 (triggered by bytecode cache)
14:56jonconaveed: can we do our meeting in 15 mins?
14:57naveedjonco: sure
14:57jonconaveed: thanks
15:11jonconaveed: ok in your room now
15:20jorendorffYoric: I'd like to talk about error handling
15:20jorendorffYoric: if you have a moment, that is
15:21jorendorffoh, rats, it's an hour later than I thought
15:21jorendorffYoric: it can wait, i'll leave comments in the code review
15:36jandemehsan: fwiw i sent this to ben earlier,
15:36jandemehsan: worth moving into mfbt maybe
15:37ehsanjandem, __builtin_sadd_overflow isn't new though :)
15:37ehsanand this isn't jit
15:37jandemthis is what the interpreter uses for instance for int32 + int32
15:37jandemi agree :) but it might be nice to use this for CheckedInt?
15:37ehsanthat commit added __builtin_add_overflow and friends
15:37ehsanjandem, yes absolutely
15:38ehsanjandem, the mfbt bug has tried to do this but it has languished
15:38jandemehsan: ah i didn't look at that bug yet
15:38bkellysorry for my confusion here
15:39ehsanjandem, needinfoing mats
15:39ehsanbkelly, np, glad you asked, I bet many people would have the same question :)
15:40ehsanjandem, why does spidermonkey not use CheckedInt btw?
15:40bkellydo we not need things like multiplier overflow? I don't see those in searchfox:
15:40jandemehsan: no this code is not that old, bug 1114782
15:40bkellyof, its smul
15:40firebot FIXED, Use compiler builtin overflow checking functions for SafeAdd and friends
15:41jandemehsan: we do use CheckedInt in a lot of places in spidermonkey, not sure why this wasn't added there
15:42ehsanbkelly, CheckedInt does multiplication validation also so it could use those too
15:42ehsanjandem, I'd be careful about using more CheckedInt for now, its perf sucks badly
15:43ehsanjandem, no solutions on windows for one thing :(
15:43bkellyehsan: clang on windows!
15:43ehsanbkelly, yes :)
15:43jandemehsan: inline assembly? :P
15:43ehsanjandem, yeah, we should really do that
15:44ehsanjandem, I have zero hopes of mfbt peers accepting that though
15:47Yoricjorendorff: Comments in the code review sound good.
15:51Yoricjorendorff: And I fully expect error handling (and, well, so many things) to change. But I needed to get it started somewhere :)
15:52jorendorffno worries
16:20tcampbelljandem: looking at the ComputeImplicitThis bug again, I think I want to change GetThisValue to have UndefinedValue for any environment we don't whitelist. No current bugs, but seems a good spot to sanity check for environment leaks
16:22jandemtcampbell: sounds good to me, best to be conservative with these things
16:23tcampbellhaving a missing |this| seems like a much better error than the opposite
16:24joncoso what's the difference between DEBUG and JS_DEBUG?
16:26sfinkI can never remember. I think one is intended for things that change data structures, so that you could eg compile wiith DEBUG off and link to a spidermonkey that was compiled with DEBUG on?
16:27sfinkbug 939505
16:27firebot FIXED, problems embedding against a debug build
16:28sfinkespecially worth reading for the final bullet points in a couple of jorendorff comments
16:30tcampbelljandem: Can you do a quick glance at . I don't know how to re-review on mozreview =\
16:31jandemtcampbell: give me 40 minutes
16:32tcampbelljandem: no worries, I'm rerunning tests so it wont go in for at least an hour
16:34joncosfink: cheers
16:35nbpsfink: so you we never use JS_DEBUG in jsapi / jsfriendapi?
16:36sfinkwe should probably always use JS_DEBUG in those files. We should really switch entirely to JS_DEBUG, it seems from reading the bug.
16:39nbpsfink: ok, I thought this was to be able to use a in place of a
16:39sfinkit is
16:40nbpso API structures should have the same layout in all cases, if they are exposed in jsfriendapi or jsapi.
16:40sfinkif you compile your embedding while #including jsapi.h with JS_DEBUG set, you'll get the right symbols and structures for linking against mozjs-debug, even if your embedding is not compiled with DEBUG.
16:41sfinkyou set JS_DEBUG appropriately depending on whether you want to link against mozjs-debug vs mozjs-opt
16:41sfinkand that should work regardless of whether DEBUG is defined when compiling your embedding
16:41nbpah, so it is not replacing a library by another one.
16:42sfinkif API structures need to differ between debug and non-debug, then the difference needs to be controlled by JS_DEBUG
16:42sfinkI'm not sure what you mean by replacing a library
16:43sfinkgcc -DJS_DEBUG -lmozjs-debug should work. gcc -UJS_DEBUG -lmozjs-opt should work. But any other mixture won't.
16:43sfinkand this is clearly documented in a hard-to-find bug ;-)
16:44sfinkI have no idea how well or badly we're doing; I don't think any of us are aware of JS_DEBUG, so I'd guess it rots all the time.
16:45sfinkoh, right, and I think the final piece is that js-config is a script that outputs the appropriate -D defines for whichever library you have.
16:46sfinkand none of this seems to be mentioned in
16:49sfinkoh, hm, for the compile, you'd #include "js-config.h" in your embedding.
16:49* sfink updates the above wiki page, at least
16:50sfinkoh, except you don't, because jsapi.h includes jspubtd.h includes jstypes.h includes js-config.h
16:50* sfink is learning, slowly
16:52sfinkI guess you don't need js-config; you'd just have separate directories for the debug vs opt packages, and -lmozjs with the right path set. Then you'd get the right js-config.h via the includes path, and the right library via the library search path.
16:58nbparai: Thanks for adding documentation for JSMSG_DEPRECATED_FOR_EACH, it appeared ~700 times in the devtools console of the last 3 beta & nightly versions :)
16:59nbp^ last 1.5, because we have a telemetry gap :/
17:31jandemtcampbell: GetThisValue changes look good to me,
17:32jandemtcampbell: what is the non-env/non-global case though?
17:32jandemtcampbell: is that NSVO?
17:32jandemor no that's an env
17:32ptomato-Msfink: nbp: there's also bug 1261161 on this topic, I didn't know about the one with the bullet points you linked above
17:32firebot NEW, MFBT/SM's use of DEBUG is a pain for SpiderMonkey embedders
17:32ptomato-MFWIW, it breaks pretty badly if you don't define/undefine DEBUG correctly
17:33tcampbelljandem: I explictly added NSVO in a followup tweak. The object case was related with WithEnvironment targets, but you are right that it may need review.
17:34ptomato-Min GJS I have this...
17:34tcampbelljandem: How about I pull the patch for now, and sanity check the object case. I also want to check on DebugEnvironmentProxies
17:34jandemtcampbell: ok i was thinking it would be nice if GetThisValue could assert IsEnv || IsGlobal
17:34ptomato-M...and this
17:34jandemtcampbell: SGTM
17:34tcampbelljandem: can you pull your review from reviewboard? I don't think I can
17:36tcampbelljandem: we don't handle debug proxy in the current code, and I think I'm hitting a related issue in the browser chrome debugger
17:36sfinkptomato-M: ouch. I'm so, so sorry. :-|
17:37jandemtcampbell: ok done
17:37ptomato-Mthere's a comment on the bug I linked about mfbt/Vector.h that I don't understand
17:37jandemtcampbell: i was just checking if is<EnvironmentObject> includes the debug proxy but apparently not
17:38ptomato-Motherwise this is probably a patch that I could write myself...
17:39tcampbelljandem: nope. I&#39;ll do some review of our uses and see if we need to do something
17:40sfinkptomato-M: the problem is that mfbt/ is shared with gecko, and gecko doesn&#39;t really make any attempt at embedding. In particular, it uses DEBUG all over the place. We&#39;d probably need to convince a build peer to switch to something like MOZ_DEBUG.
17:41sfinkthough I&#39;m not sure what MOZ_DEBUG means today
17:41ptomato-Moh, I see
17:41sfinkthen we&#39;d probably want to globally replace JS_DEBUG with MOZ_DEBUG
17:41sfinkglandium: ping
17:43sfinkcurrently, MOZ_DEBUG does not appear in js-config.h, but that might just be a matter of adding it.
17:44sfinkand clearly, if we *do* manage to fix this up, we need to make SM(pkg) or some other job test it by cross-linking and running some basic tests
17:45sfinkthe current situation definitely has a &quot;no, we don&#39;t *really* want you to embed spidermonkey&quot; vibe
18:09ptomato-Msfink: compiling and linking a minimal program is a really good idea IMO
18:10ptomato-Mthis one might be a good candidate
18:12bzIf I just want to run browser rooting analysis on try...
18:12bzIs &quot;try: -b do -p linux64-haz -u none -t none&quot; the right thing?
18:12sfinktry: -b do -p linux64-haz
18:12bzperfect, thanks
18:13sfink(it won&#39;t spawn any unit tests regardless)
18:23sfinkglandium: unping, needinfo&#39;d on bug 1261161 instead
18:23firebot NEW, MFBT/SM&#39;s use of DEBUG is a pain for SpiderMonkey embedders
18:29Yoricnbp: Ok, took the opportunity to fix `out.printf(&quot;bla&quot;)` => `out.put(&quot;bla&quot;)` & co.
18:56nbpYoric: damn, I was thinking of making a big sed for that :P
18:57YoricAlready done :)
19:17Yoricnbp: Oh, fun, oh joy:
19:19nbpYoric: everything is explained at the end.
19:19nbpYoric: sorry about this :/
19:19YoricYou mean the macro `putc`?
19:20Yoricnbp: ^
19:20nbpYoric: yes, maybe remane it to putChar()
19:20YoricYeah, good idea.
19:22nbpYoric: lol, out.put(&quot;&quot;)
19:23nbpnice one!
19:32Yoricjorendorff: Ok, I have a few questions for you (in the bug) before I can proceed to fix everything you dislike about my code :)
20:10Yoricnbp: Do I need to make vm/Printer.h somehow public?
20:38djvjnaveed, Navid:
20:51tcampbelljorendorff: ping
21:02jorendorfftcampbell: I know, I will work on it today
21:03tcampbelljorendorff: sorry, didn&#39;t mean to pester. We&#39;re trying to make decision on what amount of it is landing in 57
21:03jorendorffYoric: looking
21:03jorendorfftcampbell: it is always 100% fine to pester me
21:04jorendorffi haven&#39;t even looked yet but I will comment in the bug before i sleep
21:04tcampbellsounds good. thanks :)
21:06jorendorffYoric: replied
21:13jimbI really like the new Scope stuff, separating out the static scope chain
21:14jimbit makes me so happy to see it cleanly segregated out
21:18TitanNanothe ECMAScript spec says &quot;Perform EnqueueJob(&quot;PromiseJobs&quot;, PromiseResolveThenableJob, promise, resolution, thenAction ).&quot; which seems to me that promise then() callbacks should be appened to the job queue as soon as the promise gets fulfilled. While Chrome does that, Firefox 56.0b9 (64-bit) synchronously executes the callback function.
21:18TitanNanois this a valid behaviour?
21:18araido you have testcase?
21:20TitanNanohmm yes let me build a jsFiddle real quick
21:20araithanks :)
21:38TitanNano so while building this I noticed that my issue only occurs when stepping through the code with the debugger. if the code is executed normaly, all console logs appear in the right order, but if I step through the code line by line, the promise callback is executed synchronously and the &#39;stage x has been....&#39; log appears after the second stage has been finished.
21:39TitanNanodoes this mean the debugger is to blame and the devtools team should fix this?
21:46arailet me check
21:47araione thing to mention is that, console.log happens asynchronously
21:48araiso, relying on it for the execution order might be misleading
21:49TitanNanoI see, but when I step through the code the debuger goes into the callback before it jumps to the line with the console.log below the promise.
21:49araiwhere should I start stepping?
21:50TitanNanoI started at line 65 of the fiddle
21:53araiindeed, it works differently
21:53araiit would be nice to ask in #devtools
21:54araiI thought, I saw some change about tracing promise chain execution recently
21:57TitanNanookay, I will get in touch with them
8 Sep 2017
No messages
Last message: 13 days and 7 hours ago