mozilla :: #jsapi

11 Sep 2017
07:48jandemhm Austin, nice
08:29mrgigglesthe mozilla-inbound tree is now closed (gecko decision task bustage from bug 1342392 )
08:29firebot NEW, Migrate funsize to TaskCluster graphs
08:40mrgigglesthe mozilla-inbound tree is now open
10:05nbptill: If you want to book a room for January's event do it sooner rather than later, I heard that our Common room is quite booked until January, no later than Wednesday.
12:36evilpieany idea how to skip tests in jstests when not running nightly?
12:38anbaevilpie: `skip-if(release_or_beta)` in the test header?
12:38evilpieanba: thanks! I was searching for nightly ..
12:39RyanVMjimb added a newer function for that
12:41evilpieRyanVM: that doesn't help me, this is for jstest that we can't modify (imported from test262)
12:41evilpiebut neat helper
12:41RyanVMyeah, better than adding guards we never remember to remove later
12:42evilpieanba: btw do you know what's up with the test262 updates?
12:42evilpiestill happening?
12:43anbaevilpie: No idea what's going on there. leobalter and sphink were doing some test262 work, but I don't know the current status there.
12:44anbaevilpie: bug 1374290 (also contains patches to update test262 files)
12:44firebot UNCONFIRMED, imports Pull Requests
12:45evilpieoh I think I was looking for that .. didn't block the test262 meta bug
13:07Yoricjorendorff: Incidentally, I hate you. The fact that I need to use FullParseHandler instead of calling myself `new_<...>` means that I actually need to make sure that all my tokenization offsets-for-error-messages are propagated correctly, which is something I&#39;d gladly have postponed.
13:15jorendorffYoric: :(
13:15jorendorffYoric: well, it doesn&#39;t have to be _correct_
13:15Yoricjorendorff: Well, it needs to pass quite a few assertions :)
13:16YoricAh, well, I guess I had to make it work at some point, eventually.
13:16jorendorffHmm. That&#39;s actually a bit troublesome, since the assertions that work for &quot;the JS language&quot; format may not be the same for the BinAST format
13:25jorendorffHmm. We don&#39;t want to parameterize the whole ParseNode class hierarchy based on the location-information-type
13:27jorendorffOne fix, then, is to move all the assertions to the FullParseHandler, and assert only if a bool FullParseHandler::assertLocationSanity_ is set
13:42Yoricjorendorff: I&#39;ll see if I can make this work. Looks like a longer patch than I hoped for, though.
13:42jorendorffok :-\ sorry
14:21Yoricjorendorff: As far as I can tell, I can get away with deactivating/moving a single assertion. So that shouldn&#39;t be too hard after all.
15:26tcampbellstandups: More XPConnect fun (Bug 1398601)
15:26standupsOk, submitted #50734 for
15:38tjrOkay, I&#39;m going to try debugging some JIT code on Win64. I have a Debug build off treeherder and I&#39;m going to run firefox with ion-eager (couldn&#39;t find documentaiton ubt I&#39;m guessing this JITs everything?), disabling e10s, attaching to the parent process and breaking on EnterIon
15:39tcampbelltjr: --ion-eager is for jsshell
15:39tcampbelltjr: do you need to run full browser for your initial experiments?
15:40tjrProbably not, but if I have a treeherder build I didn&#39;t think i had jsshell
15:43tcampbelltjr: in treeheader the job is SM(p) or SM-tc(p), but I see that we don&#39;t build 64-bit windows variants
15:43jandemgsnedders: fwiw last i looked into recursion limits on Windows (2 years ago), I noticed Edge had a 10 MB main thread stack
15:44jandemlast time*
15:44tcampbelltjr: if you leave out --ion-eager, and start browser, at least a few things should still hit EnterIon
15:44tjrUnfortunetly I have to do 64bit, I&#39;m trying to debug an off-by-one error in some 64 bit assembly:
15:45nbptjr: we have a few environment variable prefixed by jitOption_...
15:45nbptjr: you can tune internals of the compiler
15:45nbptjr: for the eager compilation mode, there is a hidden flag in about:config
15:46nbptjr: you have a JS Shell, or a browser?
15:46tjrnbp: browser
15:47nbptjr: ok, let me find this about:config flag
15:47tcampbelltjr: ah, those look wasm problems. I don&#39;t think that hits EnterIon
15:48nbptjr: javascript.options.ion.unsafe_eager_compilation
15:49nbptjr: add this flag in about:config, and this should turn on eager compilation.
15:49tcampbellohh.. this is the constant blinding work.. I see
15:53marconbp: when I find two DAs in a single lcov entry, should I sum their counts?
15:53marcoe.g. your example in bug 1198356
15:53firebot NEW, SrcNoteLineScanner reports multiple times the same line.
15:54tcampbelltjr: quickly glancing at the patch, my first instinct would be to make sure that you aren&#39;t polluting flags that moveq previously did not
15:55nbpmarco: I honestly do not know.
15:56nbpmarco: I guess you can go both ways.
15:56tcampbelltjr: I&#39;d bet that is your bug
15:57nbpmarco: you could go either with a &quot;sum&quot; or a &quot;max&quot;
15:57tcampbelltjr: The addq would clobber the carry flag, and that would give an off-by-one
15:58nbpmarco: Are these DA following each others?
15:58marconbp: in the case I&#39;ve just noticed, they are not following each other
15:59nbpmarco: ok, this sounds like a bug in the bytecode emitter.
15:59nbpmarco: do you have a js shell in hand?
16:00nbpmarco: in the JS Shell, there is the dis() function, if you call it with a function it should display the line computation.
16:00tjrtcampbell: I&#39;m searching but what do you think the best approach to fix it is? Is there an instruction to add without affecting the flag? We&#39;ve been trying to do this patch without using a scratch register, so saving and restoring the flags seems difficult without that...
16:00marconbp: nope; but this report was generated a while ago, so I need to find another way to reproduce the problem
16:02nbpmarco: Open a bug againt the Core::JavaScript Frontend, with the faulty function and mention that there is a non-linear location in the source note. CC me, and maybe jorendorff.
16:03marconbp: ok!
16:03nbpmarco: the code coverage algorithm is walking the bytecode linearly, and assumed that all lines are visited linearly. If we have multiple time a line being reported, this implies that this srcnote assumption got broken.
16:04tcampbelltjr: off the top of my head I don&#39;t have an answer. (I think) moveq is used for a lot recovery stuff, so not being able to use without touching flags is likely to be problematic
16:05marconbp: it looks like it was in the top-level of
16:06tcampbelltjr: does it have to be add? is an or sufficient?
16:06nbpmarco: that is fine as well, just give the perma-link then ;)
16:06tjrNo I think an xor would work....
16:11tcampbelltjr: looking at it, it seems xor does work
16:12tjrtcampbell: I fired off to give it a shot!
16:13tcampbelltjr: I&#39;d say draw out the dataflow and see if it makes sense. Also, check none of those touch flags. And a helpful comment about the dangers of touching flags :p
16:59mrgigglesthe mozilla-inbound tree is now closed (spidermonkey failures from bug 1385278?)
16:59firebot NEW, Move some functions in CacheIR from Ion
17:09mrgigglesthe mozilla-inbound tree is now open
17:12Yoricstandups: Bug 1397717 - Move *::dump() to GenericPrinter - v3
17:12firebot NEW, Move *::dump() to GenericPrinter
17:12Yoricstandups: Bug 1377007 - Implement BinJS-ref decoder in SpiderMonkey - v2
17:12firebot NEW, Implement BinJS-ref decoder in SpiderMonkey
17:18standupsOk, submitted #50738 for
17:18standupsOk, submitted #50739 for
18:29tjrtcampbell: So I got a new off by one error. Old was got 305419895, expected 305419896; new is got -305419897, expected 305419896
18:29tjrIt looks like shift can affect the carry flag.
18:30tjrHowever there is a rolx ASM instruction that i ought to be able to use without affecting any flags
18:30tjrBut i don&#39;t think we have that implenented in the jit...
18:35mccr8tcampbell: John-Galt: I have tweeted about the JSM sharing stuff. Hopefully people trying it won&#39;t turn up a bunch of new issues.
18:38tcampbellmccr8: well, finding issues is what we need. I&#39;ve got patches up for the subscript loader issues. The IndirectEval is still open
18:38mccr8sure, if there are issues, I&#39;d like to find them. I&#39;m just hoping there aren&#39;t any. ;)
18:40tcampbellmccr8: do we have a plan for enabling flag? Just turn it on for everyone in nightly and watch crashstats?
18:41tcampbellI&#39;m glad that one reported noticed the caching issue early.
18:41mccr8tcampbell: yeah I think that&#39;s okay. No real need for a pref. If it causes stability problems I think we&#39;ll notice.
18:41mccr8I wasn&#39;t even thinking about that part of the loader. So yeah it is good somebody found it.
18:44tcampbellmccr8: hmm.. QF triage just updated us to not turning on in 57
18:44John-Galtmccr8: Nice. It would be good to try to confirm how many people the user pref flip actually works for. It never works for me in the main process. I haven&#39;t had a chance to figure out why yet.
18:44mccr8John-Galt: really? Hmm that&#39;s not great. I should check.
18:45mccr8tcampbell: At this point I&#39;m guessing that&#39;s more like they aren&#39;t going to consider it a release blocker...
18:45tcampbelloh, make sense
18:45mccr8John-Galt: hmm same for me...
18:46John-GaltI think there might just be some uncertainty about the timing of the pref service initialization. We can probably just force it when we check the pref, if necessary
18:47John-GaltAnd maybe do what I suggested with the pre-loader compilation scope so it happens a bit later
18:48* John-Galt still considers it a soft release blocker, given the perf numbers
18:49John-GaltIt makes a huge difference when webextensions do crazy things like query the list of tabs a thousand times or add dozens of different request observers. Ideally, they should just behave themselves, and we wouldn&#39;t have to worry. In practice, though...
19:10tcampbellthe subscript loader within JSM case requires 6 distinct non-syntactic environments =\
19:18John-GaltYeah, it is unpleasant. I think what we really want is: 1) Env chains either capture both qualified and unqualified assignments, or neither. 2) If they capture qualified assignments, scripts must use strict mode, so `this` boxing is not an issue. 3) Hopefully some sort of consistent `this` behavior?
19:22tcampbellJohn-Galt: I&#39;ll move the cross-compartment check back. My first (and incorrect) attempt at a diagnostic needed it, but right now the source compartment isn&#39;t needed
19:23John-GaltAlso Cool
19:23tcampbellI&#39;d prefer the FrameScript doesn&#39;t get NSVO now because that is inconsitent with current behaviour
19:23tcampbellthat sounds more like follow up
19:24tcampbellstep 1 is document madness
19:25John-GaltYeah, I&#39;m OK with that for now. I still think the current behavior is a bug, but we can fix it in a follow-up
19:28* John-Galt glances at bug 1363200, notes that jorendorff could have just deleted the add-on scope/interposition stuff rather than moving it somewhere else.
19:28firebot REOPENED, Change JSAPI to distinguish realms and compartments
19:29tcampbelloh, agreed it is a bug. In the immediate term it causes crashes because &quot;IsJSMEnvironment&quot; matches for the NSVO and tripped assertions later about whether the lexical environment is preserved or not =\
19:30John-GaltAh. :(
19:30* tcampbell probably should know what is happening with the Realms work
19:33ptomato-Mwhat is js::ScriptEnvironmentPreparer actually for?
19:36tcampbellptomato-M: some Gecko CycleCollector magic that I don&#39;t understand
19:37ptomato-MI&#39;m trying to add a debugger to GJS and I ran into an assertion failure because I didn&#39;t have a script environment preparer set
19:37ptomato-MI cargo-culted the one from the JS shell and it seems to work fine, but I&#39;d like to understand it
19:38ptomato-Madditionally, it&#39;s a bit annoying to have to inherit from a JSAPI class, because I have to make sure to match SpiderMonkey&#39;s RTTI compilation setting
19:42tcampbellptomato-M: looks like doing it like the shell is fine. It seems used by DOM in to use a JSContext containing a lot of other DOM info
19:53mccr8John-Galt: do you think I should tell an addon person that I&#39;m going to remove __exposedProps__? gabor was concerned that removing it might interfere with legacy addon porting work.
19:56mccr8I&#39;m not entirely sure who&#39;d I&#39;d even mention that to.
19:58John-Galtmccr8: You may as well add the addon-compat tag to the bug, but it shouldn&#39;t really matter now. I suppose it&#39;s just barely possible that some newer content scripts may use it, though.
19:58mccr8alright, thanks.\
20:01tcampbelljorendorff: I apologize in advance for the horrors that patch is trying to address
20:23tcampbellJohn-Galt: do you have an explanation for the tabpaint regression with jsm global sharing?
20:24John-Galttcampbell: Nope. I was wondering about them too, though. One possibility is that other things happen faster during startup so wind up conflicting with the tab paint timing.
20:27tcampbellthat would not surprise me. I think nbp had similar mysteries with the bytecode cache were ordering of event changed because some now ran quicker
20:32jorendorfftcampbell: one of these patches adds .setNoScriptRval on two tests in js/src/jsapi-tests/testExecuteInJSMEnvironment.cpp
20:33jorendorfftcampbell: why is that needed?
20:33jorendorffstamped regardless
20:34tcampbelljorendorff: Added an assert in ExecuteInJSMEnvironment since it isn&#39;t plumbed for return values and gecko also uses that option
20:35jorendorffthis is fine
20:35tcampbellI probably could have spun that answer a little better
20:35tcampbellIt was more in the lines of assert-the-world for things we know gecko is trying to avoid
20:36jorendorffit&#39;s surely better to have the assertion than not to have it.
20:38tcampbellYou are right that this has gone too far without API comments. Will fix that
20:41tcampbelljorendorff: unrelated. What is the rough plan for multiple realms? Is it just that multiple globals are in the compartment and the JSContext can only have one active?
20:41jorendorffYes, pretty much.
20:41jorendorffThe other weird thing is that every object has an associated realm... except for cross-compartment wrappers, which do not
20:42jorendorffSo the new-world replacement for JSAutoCompartment is going to have to have some special corner case for when the argument might be a wrapper. :-\
20:44tcampbellsounds like fun. After that, do we get to roll back all the JSM shared-global stuff? :p
20:45tcampbellI guess shared-global still gives the memory saving
20:46mccr8the shared global stuff does have the advantage of not having a billion copies of object prototypes etc.
20:46mccr8but I don&#39;t know how much of the memory savings is that vs the CCWs
20:47tcampbellunclear. I think a lot of the prototypes are lazy, but the working set still uses memory
20:53John-GaltA lot of them are lazy, but we still use a ton of them in every module.
20:53John-GaltI suspect CCWs are the larger part, though.
20:54John-GaltAlso, I&#39;ve seen initialization of BackstagePass scopes show up a lot in startup profiles. Just defining the default set of global functions on them takes ages, in part because each one requires a ResolveProperty call on the XPCScriptable for the global.
20:54John-GaltWhich we could fix by moving those functions to WebIDL, but that&#39;s more work.
21:05mrgigglesthe mozilla-inbound tree is now closed (xpcshell failures)
21:20tcampbelljorendorff: added doc comments to the API:
21:20tcampbellroughly what you imagined?
21:22tcampbell(just the jsfriendapi.h)
21:45jorendorfftcampbell: looking...
21:54djvjnaveed: ping
22:08jorendorfftcampbell: sorry, distracted, really looking now
22:10jorendorfftcampbell: is it possible for JSOP_GLOBALTHIS to occur in a function script, or JSOP_FUNCTIONTHIS to occur in a non-function script?
22:11jorendorfftcampbell: ok, that&#39;s what i thought -- in that case there&#39;s a comment in some code here that should be tweaked, i&#39;ll post a re-review
22:12tcampbelljorendorff: ah, I was thinking about if the script is global but calls functions in its environment, they will have JSOP_FUNCTIONTHIS
22:13jorendorffwhat i was thinking was wrong
22:14jorendorffi was thinking that plain old `with` statements also affected GLOBALTHIS but not FUNCTIONTHIS. not so
22:14jorendorffthey affect neither, it seems
22:14tcampbellthe comment *should* give the sentiment that we need all these extensiblelexicalenvironments for a reason
22:15tcampbellplain old &#39;with&#39; only affects IMPLICIT_THIS
22:15tcampbellbecause otherwise our jobs might be pleasant
22:17tcampbellonce this JSM stuff sticks, I&#39;m gonna need to overhaul the EnvironmentObject.h doc comment
22:18jorendorfftcampbell: ok, posted
22:19jandemtcampbell: globalthis can appear in arrow functions
22:19jandemat least when i added it
22:19tcampbelloh right. yes
22:19* tcampbell thinks if I made any assumptions about that...
22:22John-Galttcampbell: It actually looks like the tabpaint regression isn&#39;t real. The graphs show that it&#39;s about the same as other recent Windows tabpaint try runs, which are all slower than runs on inbound.
22:22tcampbellJohn-Galt: great news
22:25mrgigglesthe mozilla-inbound tree is now open
22:31tcampbelljorendorff: thanks for the comment block. It actually looks correct as is
22:33tcampbelljorendorff: I&#39;ll probably run some bad ideas by you tomorrow about making &#39;new Function&#39; optionally per-JSMEnvironment.
22:33jorendorfftcampbell: heh, sounds great
22:33tcampbellI&#39;m putting together the prototype now, but it may be way too hacky
22:33tcampbellWill be a fun exercise though :p
23:24mrgigglesthe mozilla-inbound tree is now closed (Various failures)
12 Sep 2017
No messages
Last message: 8 days and 15 hours ago