mozilla :: #content

17 May 2017
01:58bkellybz_away: would it be reasonable to place a limit on the firing depth in setTimeout()? basically a limit on how many sync loops we can nest before we stop firing timeouts
01:59bkellyit would be nice if we could limit it to 256 or something
11:45smaugbaku: why do we store worker name as CString?
11:46smaugwhen spec uses normal DOMStrings
11:46bakusmaug: no particular reasons. I just kept the current setup. I can change it to nsString and avoid several NS_ConvertUTF8toUTF16.
11:47smaugyeah, I think that would make sense
11:47smaugI wonder why we started to store name as CString initially
11:48smaugthere must have been some reason
11:49bakuyes, that but that is called once.
11:49bakuName() can be called several times
11:51smaugyeah, nsString would be nicer
11:53bakusmaug: separate patch
11:54smaugask review again then for the other patch :)
12:36smaugbaku: why is MOZ_ASSERT(IsSharedWorker()); removed from WorkerName() ?
12:37bakubecause now WorkerName() can be called also by Dedicated workers
12:37bakuyou can do : new Worker(script, "foo"); and in script you can do:;
12:43smaugbaku: how could that method called from DedicatedWorkers?
12:43smaugI'm talking about the method in WorkerPrivateParent
12:43bakuwhen DedicatedWorkerGlobalScope is created, WorkerName() is used
12:44baku+ globalScope = new DedicatedWorkerGlobalScope(this, WorkerName());
12:46smaugbaku: what is this mServiceWorkerScope ?
12:46smaugwhy is that needed?
12:46bakuit's needed because ServiceWorkerEvent wants that
12:47smaugand it needs to be nsCString?
12:48bakuGetRegistration and SWM and so on work with nsCString, yes.
12:52smaugbaku: where do we pass non-NullCString as to constructor as ServiceWorkerScope ?
12:53bakusmaug: I don't get the question
12:53smaugbaku: you add a new param
12:54bakuyes, for DedicatedWorkerGlobalScope
12:54smaugto Constructor
12:54bakuah yes
12:54smaugand pass NullCString()
12:54smaugwhere do we pass non-NullCString()
12:55bakudom/workers/ServiceWorkerPrivate.cpp in the second patch.
12:56smaugoh, and name is null
12:56bakuyes, because for ServiceWorker we don't have a name, just a scope.
12:57bakubefore WorkerName was used in this way: SharedWorker->name, DedicatedWorker->null, ServiceWorker->scope
12:57bakubasically 3 meanings in 1 string
12:57smaugThis is a bit confusing when our member variable handling doesn't map to what the spec has
13:01smaugbut I guess it is better now after the patch
13:02smaugsince we aren't misusing name for scope
13:22bkellysomeone moved my refresh button
13:22bkellywho moved my cheese
13:24* bkelly considers ripping MozPromise out of his code.
13:33bkellyfroydnj: do you know what I am supposed to use instead of AbstractThread::GetCurrent() now?
13:36froydnjbkelly: no. I don't fully grok jw's explanation for why GetCurrent should go away, either
13:36bkellyfroydnj: I'm really kind of upset about it, but I feel like I'm in the minority and I'm giving up pushing back
13:37bkellybut I don't know how to replace it
13:37bkellyin my mega-patch
13:37froydnjunless GetCurrent() would return something bogus if you weren't actually excuting in an abstract thread (promise?)
13:37mystorbirtles: AbstractThread::GetCurrent is used for labeling?
13:37mystorbillm: ^
13:37mystoroops, sorry birtles
13:39mystorperhaps smaug would know?
13:39bkellythe idea that you will always have an explicit thread to specify seems unrealistic to me...
13:39bkellyparticularly if you want to write code that works in both main thread and worker threads
13:39mystorbkelly: Are you wondering about jw's AbstractThread::GetCurrent changes?
13:39bkellymystor: yes
13:40mystorbkelly: I am confused exactly as to what this is changing
13:40bkellymystor: they are removing AbstractThread::GetCurrent()
13:40mystorbkelly: What's the motivation for that?
13:40bkellyI'm probably not a good person to ask that question
13:41bkellyI have code, though, that can be called from main window or on worker scope... if I can't do GetCurrent() any more I'll have to write some helper routines to extract it from the global object
13:41smaugmystor: I've never quite understood why we have AbstractThread concept
13:42smaugI think we should get rid of that
13:42bkellysmaug: thats a different problem... but yes, it should never have existed at all IMO
13:42mystorsmaug: I sorta mapped it in my head whenever I saw it to the "Threads" we're scheduling onto real threads for scheduling
13:42froydnjbholley_mobile explained it to me at whistler...but I cannot recall the reason
13:42bkellytwo threading ecosystems just makes it harder to maintain the tree
13:42mystorsmaug: I think it's a Quantum DOM thing
13:43smaugdefinitely no
13:43mystorsmaug: oh, ok
13:43smaugit was added for some media stuff
13:43bkellyfroydnj: he wanted to make dispatch infallible (but it can't really be) and he wanted to enable a different semantic ThreadGroup... he could have done that with a subclass of nsIEventTarget, though
13:43mystorsmaug: I saw
13:44smaugmystor: some stuff has been added there sure to be able to use AbstractThread with QDOM
13:44mystorsmaug: Which mentioned DocGroup::AbstractThreadFor which made me think it was scheduling related
13:44bkellyI just feel like the threading primitive changes are being roto-tilled and maybe not completely thought out
13:44mystorsmaug: OK - so dispatching an event to AbstractThread::CurrentThread doesn't label it automatically?
13:44bkellyanyway, I'll try to work with them... but I don't know how to do that yet
13:45smaugbevis|away: would know... I need to read the code
13:45mystorsmaug: I'm being suggested a change from AbstractThread::CurrentThread to AbstractThread::MainThread and I'm worried it means that some runnables will lose labeling
13:45mystorthey may have never been labeled if I was doing the wrong thing before
13:46smaugmystor: what code are you hacking?
13:46mystorIt's just the nsPermissionManager runnables
13:46smaugwhy does that use AbstractThread :/
13:46mystorsmaug: It uses promises
13:47mystorsmaug: e.g.
13:47smaugPromises don't use AbstractThread. You mean MozPromises ;)
13:47mystorsmaug: Yeah, those things
13:47smaughuh, typedef MozPromise<bool, nsresult, /* IsExclusive = */ false> GenericPromise;
13:47smaugsuch a weird typedef
13:48mystorI&#39;m using it as a promise which has no interesting data but just represents the completion of a promise
13:48mystorerr a task
13:48mystorSo it was being used to record when a set of permissions have arrived in the current content process
13:50mystorsmaug: I originally just used runnables and some local state but ehsan suggested that I should use MozPromises
13:50smaugmystor: so, could you use AbstractThreadFor ?
13:50bkellymystor: AFAIK GetCurrent() was not getting you labeling, no
13:50smaugtake the AbstractThread in main thread and pass it to the other thread?
13:50mystorsmaug: I don&#39;t pass to any other threads
13:51mystorsmaug: THis remains main thread bound
13:51bkellyI think one of the problems with labeling right now is there is no indication from compiler or tests if you are doing it correctly
13:51mystorMozPromise was pretty overkill
13:51smaugthen AbstractThreadFor, if just possible
13:51mystorI don&#39;t have any DocGroup or TabGroup passed in
13:52jibmystor: media::Pledge is a lighter abstraction if MozPromise is too much. Dunno if it&#39;s what you need at all, just wanted to mention it. It&#39;s used a couple of places in mediamanager etc.
13:52bkellyI feel maybe we need a helper like NS_GetThreadFor(nsIGlobalObject* aGlobal) and GetAbstractThreadFor(nsIGlobalObject* aGlobal)
13:52smaugmystor: is this possibly system code?
13:52mystorI suppose I can&#39;t really label these - I would probably want to make most of them SystemGroup, except for the final one produced by the call to WhenPermissionsAvaliable
13:52jibdoesn&#39;t do threads at all, so you&#39;ll still use e.g. runnables with lambdas
13:52mystorwhich I would want to use for the last call out
13:53mystorjib: Perhaps, I don&#39;t really want to rewrite that code today though :)
13:53jibsure just fyi
13:53mystorjib: I&#39;ll keep it on my radar for next time I need something like this
13:53mystorsmaug: Yeah, many of them should be SystemGroup
13:53mystorsmaug: But the call to WhenPermissionsAvaliable should probably take an event target
13:54froydnjjib: Pledge?!
13:54froydnjwhy not promises, or runnables? why invent a third thing?
13:55jibit came before mozPromise
13:55jiband is a lighter abstraction more like a js promise
13:55jibnot a task scheduler
13:55smaugmystor: sounds reasonable
13:56smaugfroydnj: well, MozPromise is already totally different to Promise
13:56smaugfrom scheduling point of view
13:56jibfroydnj: runnables and promises are different things after all
13:56jibmozPromise is both somehow
13:56jiban JS promises can&#39;t be used off main thread
13:57bkellyjib: they can on a worker thread
13:57jibok sure
13:57jibbut we have threads that are not worker threads
13:57jiband it seems overkill to make those worker threads :)
13:57jibcan&#39;t be used on non-JS threads I should have said
13:58smaugand MozPromises don&#39;t use microtask scheduling.
13:58smaug(not that our Promises do that yet, still working on that)
13:58* jib needs to read up on using runnables with microtask scheduling
14:00smaugthat won&#39;t be possible
14:00mystorjib: FYI, I just looked at Pledge and there&#39;s a comment to simplify it once we support std::function, which I&#39;m pretty sure we do right now.
14:00jibmystor: cool thanks
14:00smaug(there will be some microtask specific Runnable-like thing)
14:01bkellyMozPromise is a convenience for common runnable patterns... its really unrealted to DOM promises IMO
14:02bkellyI mean, we could name it Future or something to make it easier to distinguish
14:02* jib thinks it&#39;s gone through a couple of renames already
14:03bkellyyea, I don&#39;t like renames... not seriously suggesting that
14:03bkellyif it gets renamed, though, I&#39;m going to lobby for nsFuture
14:03* bkelly trolls
14:06jibmystor: I kindof stopped working on it though once mozPromise came along. I wrote it for myself initially. I could pick it up again if there&#39;s interest.
14:07jibbuy as was mentioned, having multiple patterns isn&#39;t the best
14:07mystorjib: I imagine that making it slightly easier to use MozPromise on one thread might be an easier approach. That way you only need to lean one concept. Not sure
14:07jibyeah maybe
14:08smaugbaku: do other browsers follow the spec with Option ctor?
14:08smaugah, perhaps it doesn&#39;t matter
14:09bkellysmaug: if I see awesomebar drop down janking, where do you think I should file that?
14:09jibMy main concern was I wanted a promise-like concept that was same thread, just like JS promises, so you could pass state to a then callback without needing &quot;threadsafe&quot; (thread-lock-encumbered) monitors
14:09bkellyprofile is here if your curious:
14:09smaugbkelly: Firefox: ...
14:10smaugand isn&#39;t there Awesomebar or Locationbar component
14:10bkellysmaug: looks like it could be doing a ton of layout
14:10smaugthat is where I&#39;d start
14:11smaugbkelly: getBoundingClientRect call
14:11smaugbkelly: it might be filed already
14:11smaugjust a sec
14:11* smaug looks at
14:12smaugFrontend has lots of sync reflow bugs
14:13smaugbkelly: don&#39;t see bug for _handleOverflow
14:14smaugadd [qf] to whiteboard
14:17smaugthere is also the flush when presshell handles some event
14:17smaugbut I don&#39;t think we can avoid that
14:17smaugcould make it faster though
14:17firebotBug 1365606 NEW, desktop awesomebar dropdown animation janks after launch
14:18mystorjib: Yeah, that&#39;s kinda what I wanted too
14:19jibmystor: let me know if you end up using it I guess. It&#39;s extremely lightwait, doesn&#39;t even chain atm.
14:19jibuh lightweight
14:20mystorjib: I&#39;ll look into it if I touch the nsPermissionManager code again. There&#39;s a chance we&#39;re going to be moving some of it off thread at some point to try and avoid the sync main thread IO during startup which it currently does (eek) so I might start actually needing the features of MozPromise then
14:35bkellyfroydnj: do we have a primitive to swap two integers?
14:35froydnjbkelly: mozilla::Swap?
14:35bkellyor should I just do it with a temp variable and let the compiler sort it out?
14:36bkellyfroydnj: is it worth using that if I am just swapping integers?
14:37froydnjbkelly: maybe for clarity&#39;s sake?
14:41bkellyfroydnj: thanks
14:51bkellysmaug: I guess that bug is a dupe of:
14:51firebotBug 1353725 NEW, displaying the awesomebar panel could be smoother
14:52florianHi. Is there a way in a xul window to get the equivalent of the HTML <script ... defer> behavior? It seems the main browser window currently loads plenty of scripts that aren&#39;t required for the first paint.
14:55smaugthat was to bkelly
14:57bkellythis behavior in chrome sounds really weird to me:
14:57firebotBug 1365436 UNCONFIRMED, Javascript page reload code works under Google Chrome but not Firefox
14:58bzbkelly: I _think_ what he means is that doing that get nixes the cache entry for that url
14:58smaugflorian: let me check. I think not, since XUL loading happens so differently
14:58bzbkelly: So if you later navigate to it you get a refreshed version?
14:58bzbkelly: I know there are differences in terms of how we and chrome compartmentalize cache stuff for xhr vs not
14:59bzbkelly: That said, that bug certainly doesn&#39;t say anything like that; it needs steps to reproduce
14:59bzbkelly: You want to ask for those, or should I?
15:00smaugflorian: yeah, we just create nsXULPrototypeScript and process it normally
15:00florianunrelated - I&#39;m looking for someone to either review my trivial patch in bug 1364360, or preferably take over the bug and review the whole file for similar cases (GetScrollFrame causing sync reflows when not really needed).
15:00firebot NEW, 0.96ms uninterruptible reflow at gotoPref@chrome://browser/content/preferences/in-content/preference
15:00smaugflorian: do you think defer would help with startup performance?
15:01bkellybz: I did it
15:01floriansmaug: thanks. Is there a workaround, or would it make sense to just replace script with another tag name, and then use Services.scriptloader to load these scripts after we have painted?
15:01smaugflorian: XUL loading doesn&#39;t know about defer scripts
15:02floriansmaug: I think so yes. We currently spend some time between when the browser window is created and when it&#39;s first painted evaluating all these scripts.
15:02smaugflorian: could we load these way later?
15:02smaugusing requestIdleCallback ?
15:02smaug(or on-demand if needed)
15:03floriansmaug: I think the nicest solution would be to load most of them lazily (eg. the first time we open a context menu for nsContextMenu.js)
15:03florianbut that&#39;s also a lot more work, and may cause add-on compat issues
15:03bzbkelly: awesome, thanks
15:06smaugflorian: still wondering if defer would help at all
15:06smaugreflowing and all that is different in xul
15:06smaugonly done once the document is ready
15:06floriansmaug: do you think defer would still cause the script to be loaded before the first paint?
15:06smaugI think so
15:10smauglooks like so
15:10smaugwell, xul loading is so different that we could invent some new rules, but then it wouldn&#39;t be defer
15:28floriannot sure if this is the right channel, but is there a way to control when the refresh driver timer starts? The impression I got from looking at startup profiles is that it&#39;s started after we create the hidden window, and fires a couple times before we paint the first actual window.
15:29bkellyfarre: was it an accident that Timeout::mPrincipal was added when nsTimeout it was moved from nsGlobalWindow.h to Timeout.h?
15:29smaugflorian: hmm, that is really up to the gpu process/thread and when it sends. Though, parent process needs to tell that it is interested in to get vsync
15:30bkellyfarre: I don&#39;t see Timeout::mPrincipal being used anywhere... can I nuke it
15:30farrebkelly: yes
15:31smaugflorian: do you have some profile showing that behavior ?
15:32florianI haven&#39;t saved any, but can send one to you next time I see it
15:32smaugflorian: I guess I could test myself whether I see refreshdriver ticking too soon
15:32smaugI guess nothing really prevents that
15:33floriancould we also skip GCs before the first paint?
15:34smaugat least we don&#39;t do the initial DOM triggered GCs
15:34smaugbut I guess JS engine itself my trigger GC
15:34smaugflorian: perhaps ask djvj or jonco
16:03bkellysmaug: maybe I should write my own MozPromise replacement that uses nsIEventTarget... I feel this AbstractThread stuff is getting ridiculous:
16:04firebotBug 1365483 NEW, Remove the call to AbstractThread::GetCurrent() and specifiy an AbstractThread explicitly
16:04jibbkelly: did you look at media::Pledge?
16:04bkellyjib: I need threads
16:05jibah, and runnables are too basic?
16:05jibe.g. runnables with lambdas
16:05bkellyI want the convenience of the closures
16:05bkellywell, more than that... I need to tie async stuff together... the promise concept works nicely
16:06smaugIt would be nice if MozPromises or some such worked on top of nsIEventTarget
16:09* bkelly lunches
16:09jibwell in MediaManager we use lambdas a lot for closures. E.g. NS_NewRunnableFunction([self, temp]() -> { self->foo(temp);}),
16:10jibthat&#39;s not intrinsic to promises
16:23smaugflorian: you&#39;re right, I do see a tick way before StartLayout
16:23smaugfor the browser.xul
16:24smaugthat tick probably doesn&#39;t do much
16:25jdmis <!doctype html> treated the same as <!DOCTYPE html>?
16:25jdm suggests no
16:28smaugflorian: ok, we just return early from tick
16:28smaugso, not sure there is anything to optimize here
16:28smaughiddenWindow is different beast
16:39mccr8not a good sign when blame for a line you are looking at ends up on brain transplants
16:47bzjdm: yes
16:48bzjdm: Note &quot;ASCII case-insensitive match for the word &quot;DOCTYPE&quot;
16:49bzjdm: the comments on that link of yours talk about that
16:49bzjdm: The thing failing is some validation bits on their end plus some maybe-XML process
16:49bzjdm: but in HTML they&#39;re equivalent
16:49jdmbetrayed by skimming once again
17:28bkellyehsan: can content painting jank browser chrome these days? maybe just with software rendering?
17:38floriansmaug: has 12ms spent in nsRefreshDriver::Tick before we first paint the browser window.
17:40smaugflorian: but that is after StartLayout
17:41smaugso painting should be ok
17:42florianhmm, maybe I&#39;m just confused. What&#39;s StartLayout, and why is it ~400ms before we first paint?
17:43smaugflorian: hmm, I&#39;m trying to see first paint
17:43florianit&#39;s the very end of the profile
17:43smaugah there
17:44florianwhen looking at startup profile, the first thing I do is usually to narrow down the view to have the first green rectangle at the end of the profile. That&#39;s when the first browser window becomes visible
17:46smaugthe profile hints that not ticking after DOMContentLoaded would just move the same amount reflow time to happen before the first paint
17:46smauglet me try something
17:47florianthat&#39;s assuming the JS code that runs in the middle isn&#39;t going to touch the DOM much though, right?
17:54smaugflorian: I just don&#39;t think we know currently when the first paint is supposed to happen. Just that it is after StartLayout(). And StartLayout() is right before DOMContentLoaded
17:54smaugbut perhaps we could improve this...
17:55florianthere&#39;s also 2 cheap (1 or 2ms) nsRefreshDriver::Tick calls earlier in that profile
17:56smaugflorian: your testcase is to start browser?
17:56smaugwith that env variable which starts profiler too
17:57florianI have a bat script that sets the 2 env vars I need to have the profiler active at startup and then starts firefox.exe
17:58smaugflorian: 2 env vars?
17:58smaugwhat is the other one?
17:58florianMOZ_PROFILER_STARTUP_ENTRIES=<large number>
17:59florianso that my profiles still includes the beginning of startup if cold startup took a long while due to main thread IO
17:59florianI&#39;ve sometimes profiled startups that take more than 2 minutes.
17:59smaugoh, didn&#39;t know about MOZ_PROFILER_STARTUP_ENTRIES
18:09smaugflorian: assuming my patch works, want to perhaps test it?
18:09smaug(just building it)
18:09smaug(takes some time since it touches nsIDocument)
18:10florianI&#39;m not building on the reference device, so it would be preferable if you can give me a try build
18:10smaugflorian: oh, you&#39;re testing on the qf laptop?
18:10smaugI&#39;m not building on windows at all
18:10smaugI guess try build it is then
18:10floriansmaug: yes, that startup profile is on the quantum reference hardware
18:16jugglinmikebkelly: I have another event processing question. If you have the time, I&#39;d appreciate your input
18:16bkellyjugglinmike: sure
18:17jugglinmikeif the worker is terminated, it&#39;s &quot;closing&quot; flag is &quot;true&quot;, so when the subsequent Update Worker State algorithm fires an event at the workerObject, why is that actually propagated to listeners?
18:18bkellyjugglinmike: so a service worker thread is terminated and gets it &quot;closing&quot; flag set?
18:19bkellyjugglinmike: the worker thread itself is different from the ServiceWorker DOM objects exposed to other windows/workers
18:19bkellyjugglinmike: I think is refering to the DOM objects
18:20bkellyjugglinmike: but I don&#39;t see the closed flag... so not sure what you mean
18:21jugglinmike&quot;closing&quot;, via Terminate
18:21bkellyjugglinmike: right, that is on the worker thread&#39;s global... not on the ServiceWorker DOM objects refered to in
18:21bkellyI think
18:24jugglinmikeI think I understand that distinction, but I also don&#39;t think it answers the question. The &quot;closing&quot; flag is correctly set on the global, and the event is correctly fired at the DOM object
18:24bkellyjugglinmike: I guess I don&#39;t understand the question
18:24jugglinmikeBut I&#39;m still unclear about why, after the flag has been set, the subsequent event is actually processed
18:25bkellyjugglinmike: the DOM objects are owned by different globals than the one getting the closing flag set
18:25bkellyjugglinmike: which event are you referring to? can you give me an example?
18:26jugglinmikeyup, step 3.1.2 in Update Worker State
18:26ehsanbkelly, hmm, I don&#39;t remember seeing something like that... have you?
18:26jugglinmikethe &quot;statechange&quot; event
18:27bkellyjugglinmike: ok, why do you think the statechange should not fire? is there a step in an algorithm?
18:27bkellyehsan: I was seeing it today on a linux build, but I was running it on X-over-ssh
18:27bkellyand I don&#39;t have symbols setup for the profiler
18:28jugglinmikebkelly: based on my reading of
18:29jugglinmike...but now I&#39;m seeing that &quot;fire an event&quot; uses &quot;dispatch&quot;, and that &quot;dispatch&quot; is fully synchronous
18:30bkellyjugglinmike: the task in is not on that global... it must be on the target global&#39;s queue
18:30jugglinmikeoh, wait
18:30bkellymaybe the spec language is confusing here
18:30bkellyit must queue the task on the same global that owns the ServiceWorker DOM object in question
18:30jugglinmikebkelly: or maybe I&#39;m just dense. At the bottom of Update Worker State,it reads, &quot;The task must use workerObjects relevant settings objects responsible event loop and the DOM manipulation task source.&quot;
18:31bkellyjugglinmike: yea, it has to use the other globals task queue
18:31bkellyjugglinmike: now, if there was a ServiceWorker object in the service worker thread&#39;s scope... like that should not get the statechange event
18:31bkellybut stuff like window.navigation.controller should get the statechange event
18:32ehsanbkelly, hmm, X over ssh...
18:32ehsananything could happen there, I have no idea about that setup
18:32ehsanand tbh nobody really cares about linux :(
18:32bkellyehsan: I could try on my mac later... my windows machine does accelerated stuff
18:32bkellyehsan: I&#39;m more concerned it affects mac
18:33billmsmaug: ping
18:33ehsanbkelly, we don&#39;t (shouldn&#39;t be) using X on mac by default
18:33smaugbillm: pong
18:33bkellyehsan: I&#39;m not saying we should... I&#39;m saying we do software rendering on mac
18:33billmsmaug: I wasn&#39;t sure what you meant in your review about IsOnCurrentThread. that&#39;s a method that&#39;s been around for a long time. it&#39;s not something I added.
18:33bkellyehsan: and maybe it has the same problem as I observed here
18:33ehsanfor sure
18:33ehsanit&#39;s hard to say
18:34smaugbillm: oh
18:34ehsanwe also do software rendering on windows, fwiw
18:34billmsmaug: I could maybe change it, but I think it might confuse people more
18:34smaugbillm: I probably just remembered it was in nsIThread
18:34smaugoh oh
18:34smaugyes, my mistake
18:35smaugbillm: it is silly name anyhow
18:35smaugI totally thought it was in nsIThread
18:35billmsmaug: yeah, I agree that it&#39;s not great
18:35smaugbillm: ask review again
18:35billmsmaug: ok, will do. thanks.
18:36bkellyehsan: the case I was running was this: with dom.timeout.max_consecutive_callbacks set to something like 9999
18:37bkellyehsan: and using the hover effect over the reload button to tell if it was janking
18:38* bkelly pulls out his mac.
18:38ehsanbkelly, I just profiled that page on linux
18:38ehsanno jank on the parent
18:39bkellyehsan: with the pref set?
18:39ehsanbkelly, oh sorry
18:39ehsanmissed that part
18:39ehsanmy screen font size is way too small for my eyes ;)
18:39bkellyehsan: that may be fixable
18:39bkellybigger eyes, etc
18:40ehsangetting younger ;)
18:40bkellyI keep forgetting I have a mac at my desk now that I have a personal laptop upstairs
18:40jugglinmikebkelly: Thanks a bunch!
18:41ehsanholy cow does that pref change the UX!
18:41ehsantake that, servo :P
18:41bkellyehsan: well, I think servo is still better
18:41ehsanbkelly, still, zero jank in the parent
18:41ehsan(I know, joking!)
18:41bkellyehsan: the pref just lets us do more DOM changes between angonizingly slow paints
18:42bkellyehsan: yea, I don&#39;t see the jank on my mac either
18:48bkellyits super annoyign the .dmg files from try mac builds are always &quot;damaged&quot; when I try to open them
18:48bzredownloading often helps...
18:48jugglinmikebkelly: try renaming them to .fixed
18:48bzbut yes, pretty disturbing
18:49bkellybz: I&#39;ve never had that work for me
18:49bkellyI assume its some signing thing
18:52bkellybz: I had to twiddle something in System Preferences->Security->General to let me run apps that are unsigned
18:53bkellyehsan: the good news is that my time budget patches also don&#39;t jank... the patch runs similarly faster (not quite as fast as just setting the pref super high), and we stay at 30fps
18:53bkellywell, 25fps... but I don&#39;t trust devtools over much
18:54ehsanyou should use the gfx fps counter
18:54ehsanbkelly, layers.acceleration.draw-fps
18:54ehsanit&#39;s super accurate
18:54ehsan(and annoying ;)
18:56bkellyehsan: DL: ~18ms FLB: ~6ms Raster: ~8ms... fps fluctates between 16fps and 22fps
18:58bkellyehsan: without time budget and disabling our old throttling with the pref I mentioned above, we drop to ~6 fps
19:00bkellyin m-c with the flat limit at 5 timers... the animation runs super slow, but we are solidly at 30fps
19:01bkellyI guess thats not completely surprising since we allow up to 4ms of the frame to be used by RunTimeout() consecutively
19:08mystorWith the ipdl generated code is OnCallReceived the function used for RPC messages? (so like a high priority sync IPC?)
19:08mystorbillm: searchfox seems to be serving me blank pages after a decently long delay right now when I try to search for things
19:35billmmystor: it seems okay for me. what did you search for?
19:35mystorbillm: It seems to have picked up again
19:35mystormaybe it was a short outage
19:36* mystor was looking for PContentParent.cpp IIRC?
19:40billmmystor: oh, I just saw your question. high priority sync messages are distinct from RPC and don&#39;t use OnCallReceived.
19:40mystorbillm: Yeah, those are interrupt messages, right?
19:41* mystor spent some time combing through stuffs
19:41billmmystor: interrupt is the new name for RPC, yes
19:41mystorah, ok
19:42mystorbillm: What&#39;s the main difference between sync IPC and RPC, is it just message ordering?
19:42mystor(and a different path in the code)
19:42billmmystor: yeah, pretty much. ordering affects a lot of things though :-).
19:43mystoryeah - I can imagine it does...
19:43billmmystor: how messages can nest, whether async messages can run in the middle of a sync request, nested event loops, etc.
19:43billmmystor: interrupt messages are tailor-made for plugins and the nested() sync messages are tailor-made for CPOWs.
19:44mystor*shudder* cpow
19:44mystorbillm: Will those be 100% gone after 57?
19:44mystors/gone/able to be removed
19:44billmmystor: basically. we&#39;ll still be using them a lot in tests, but the code shouldn&#39;t run during normal browsing.
19:44mystorAh, that&#39;s a shame
19:45mystort&#39;would be nice to remove them completely
19:45billmyeah, it&#39;s just work that has to be done
19:45billmwe have a lot of tests :-)
19:45mystorYeah, and I&#39;m not signing up to do it
19:46billmmystor: by the way, I was looking at the new BHR dashboard. it&#39;s really nice and the data is actually believable.
19:46mystor\o/ - the one made by dthayer?
19:46mystorYeah, it&#39;s pretty awesome
19:46billmmystor: I was wondering if there&#39;s a metabug or something where bugs from that dashboard will be tracked
19:46* mystor is still working on trying to get better data out of it
19:46billmmystor: I was thinking of filing some stuff but I didn&#39;t want to end up filing a bunch of dupes
19:46mystorprobably off of
19:46firebotBug 1344003 NEW, [meta] Create (or resurrect) BHR dashboard
19:47mystorIt recently acquired a [meta] tag
19:47billmmystor: will that be a metabug for bugs filed based on the data?
19:48smaugflorian: oh, I see, the reflow before paint is coming from hiddenWindow. That is reasonable-ish
19:48mystorbillm: Bugs filed based on the data are being given the [bhr] whiteboard tag
19:48billmah, ok
19:49mystorIf you want raw data you can still use but its less nice to browse than dthayer&#39;s one
19:50billmmystor: will there be a way to get JS stacks out of doug&#39;s dashboard?
19:50mystorbillm: I hope so soon - right now there isn&#39;t but it is something which I want
19:50* mystor is looking into using the profiler&#39;s stack interleaving code to interleave the stacks being sent to telemetry
19:51billmyeah, that would be great!
19:51mystorI should probably bother dthayer to add some sort of hacky way to do it right now
19:51mystor&#39;cause that code is probably a couple of weeks out
19:51mystor(just &#39;cause I need to get time to work on it, and after that the profiler is hard to extract from needing, y&#39;know, the profiler to be running)
19:56floriansmaug: do we need layout to happen at all in the hidden window?
19:56smaugflorian: sure we do
19:57smaugI would be surprised if there isn&#39;t code injected there to be able to query size of random things etc
19:57smaugbut refreshdriver tick could be postponed
19:57florianwhy? I&#39;m not sure at all how the hidden window is used (except on Mac where it displays the menubar)
19:58smaugflorian: don&#39;t at least old style addons use it
19:58smaugkind of as a singleton
19:58florianIIRC that&#39;s a trap, because the hidden window is an HTML document on Windows/Linux and XUL on Mac.
19:58smaugI don&#39;t understand why I&#39;m still getting extra tick for browser.xul too with my patch :/
20:16sheppyAnyone know of a clean way to have a grid column span all columns regardless of how many there are?
20:16* sheppy wants something like grid-colum: 1 / span all;
20:16sheppyExcept with column spelled right.
20:19sheppyBah, never mind, I think I see the obvious answer
20:27billmmystor: what&#39;s the date range for the data in doug&#39;s dashboard? does it start on 4/18 as the top pane sorta suggests?
20:28mystorbillm: yes. I believe it is the last 30 days.
20:28mystorbillm: the data before may 2 is kinda junky though, so I might zoom into days after that
20:29billmmystor: how do I zoom in?
20:30mystorDrag over the date range in the graph and click the magnifying glass
20:37billmthanks. I guess it helps to zoom in on a date range that doesn&#39;t include the horrible ghost window regression.
20:38billmmystor: do you know what these PeekMessage stacks are about?
20:38* bkelly likes how the new irccloud &quot;midnight&quot; theme matches the mozilla favicon better.
20:41smaugbillm: so do you have some rule when you use ThreadEventTarget and when nsIEventTarget as the type?
20:42billmsmaug: the main difference is that ThreadEventTarget has a non-virtual IsOnCurrentThread method. I tried to always use that, except I didn&#39;t change existing uses of nsIEventTarget.
20:42smaugthis all is getting quite complicated
20:43smaugI wonder how to simplify this
20:43smauggetting rid of AbstractThread at some point
20:43smaugand then something ...
20:49smaugcould we merge ThreadEventTarget stuff to nsIEventTarget at some point
20:50smaugbillm: was there a reason why you didn&#39;t add the member variables to nsIEventTarget?
20:50smaugperhaps that would be messy
20:50billmsmaug: well, not all event targets would support it. just the ones for threads.
20:51billmsmaug: I think it could be done, but it would be messy and would add fields to all other event targets.
20:51smaugsure. One would need to add some if-checks
20:51smaughaving the non-xpcom version of IsOnCurrentThread on all of them would be fine
20:52billmyeah, that would be fine as long as it does the right thing
20:52smaugbillm: if mVirtualThread is not set, fallback to xpcom version or something?
20:54billmalthough we would need a way to initialize mVirtualThread to nullptr. not sure how to do that for an XPCOM interface.
20:54smaugscriptable nsIAtom.idl for example has quite low level stuff in it, as an example
20:54billmI guess we could use some C++ type with a constructor that nulls it out
20:55billmfroydnj: ping
20:55smaugnsIEventTarget could be builtinclass
20:55smaugso ensure it is implemented in C++ only
20:55froydnjbillm: pong
20:55billmoh yeah, we would have to do that
20:55billmfroydnj: smaug would like to merge ThreadEventTarget into nsIEventTarget and I wanted to see what you thought
20:56billmfroydnj: basically, we would add the mVirtualThread member to nsIEventTarget directly. it would be null for non-thread event targets.
20:56billmfroydnj: the nonxpcom IsOnCurrentThread() would delegate to the XPCOM version if it&#39;s null
20:57billmand we would have to make nsIEventTarget a builtinclass
20:58froydnjkinda think builtinclass nsIEventTarget makes sense regardless
21:00froydnjI don&#39;t think it makes sense, because the ThreadEventTarget version is infallible
21:00froydnjwhereas nsIEventTarget::isOnCurrentThread is not