mozilla :: #content

14 Mar 2017
10:42* smaug likes https://github.com/whatwg/html/issues/2432#issuecomment-286334252 . It expresses well how ShadowDOM v0 was implemented in blink :)
11:18smaugHmm, ContentPrefs is some thing which apparently needs DOM peer review, but nothing really explains what it is
14:01smaugthis one session history case is really fun. Initiating reload and fragmentnavigation at the same time and also adding some iframes dynamically
14:37bkellyanyone know if MozPromise::All() fast fails on first rejected promise or waits for all promises in the list?
14:38bkellyseems it fast fails
15:07ehsanbaku: ping
15:09bz_sleepehsan: so if we can make the value set just set the textnode bit, that would at least prevent the frame construction stuff... :(
15:09ehsanbkelly: do you use the gecko profiler at all?
15:09bkellyehsan: I used to... I've had worse luck with it lately
15:09ehsanbkelly: why?
15:10ehsanbkelly: if it doesn't work out of the box for you that's a P0 bug
15:10ehsanbz_sleep: is this about the speedometer bug?
15:10bkellyehsan: I have a harder time get the ifnormation I need out of the new cleopatra... probably I'm just not used to it yet
15:10ehsanbkelly: oh, so UI issue?
15:11bkellyehsan: I think so
15:11ehsanbkelly: ok, but still capturing profiles is a huge help to others :)
15:11bkellyehsan: I feel like I've had more trouble with symbols than I used to
15:11ehsanbkelly: I'm asking because I want to make sure you aren't avoiding using it because of bugs
15:12bkellyehsan: I feel like its more unstable lately
15:12ehsanand also because with a test case like the one you sent me I'm not 100% sure what button I should press to profile
15:12bkellyehsan: I have the plugin in my dev edition addon
15:12bkellyehsan: just profile loading the page without pressing anything
15:12ehsanbkelly: if you see bugs *please* file them, that's very important for our perf work
15:12ehsanok
15:12* ehsan does that
15:13bkellyehsan: at some point I have to focus and otherwise I'd spend all my time filing bugs
15:14* ehsan notes that timer thread lock contentions makes him so mad
15:14bkellyehsan: are you seeing that on that page?
15:14ehsanyeah
15:15ehsannot taking a ton of time
15:15ehsanbut always there
15:15ehsanfreaking performance monitoring :(
15:15ehsanalso always there :((
15:15bkellyehsan: so, I think we should go ahead and make TimeoutManager use a single nsITimer... it only needs one for the next expected timer
15:15bkellyinstead of using nsITimer for every setTimeout
15:16ehsanlet me poke that bug first
15:16bkellywould reduce our load on nsITimer a lot
15:16bkellyehsan: also, my Timer thread optimizations would help
15:16ehsanbkelly: yeah it would be really nice
15:16tbsaundeehsan: the existance of the timer thread makes me mad ;)
15:16ehsanit's not that crucial, but definitely nice to have
15:18bkellyehsan: I sent you the mail because I think html parser is something people tend to just accept as unchangeable... but we probably have some anti-jank wins we can get there
15:19bkellyunchangeable because its complicated and extremely compat sensitive
15:19bkellyanyway, I have to go shovel some snow... back in a bit
15:21Ms2gerHTML parser might be fairly complex, but also well-specified and well-tested
15:21Ms2gerI'd be much more worried about messing with anything touching docshell than the HTML parser
15:23ehsanbkelly: I refuse to accept anything as unchangeable :)
15:29ehsanbkelly: I can't find the parser related runnables that you were mentioning though, can you please ping me when you're back?
15:31smaug++bkelly about using just one nsITimer
15:33smaugI've seen profiles where setTimeout's timer handling floods timer thread
15:37smaugAwayback in couple of hours
16:40bakuehsan: pong
16:41ehsanbaku: I pinged you to talk about bug 1340163, but now I'm reviewing your other patch...
16:41firebothttps://bugzil.la/1340163 NEW, amarchesini@mozilla.com Origin-unique deviceId persistence (used in enumerateDevices and gUM constraints) is broken
16:41bakuehsan: ok
16:43padenotcan someone r+ a change to content prefs ? billm is not here it seems
16:43ehsanbaku: while I have you here
16:43ehsanbaku: why can't you create a principal on the main thread?
16:44ehsanpadenot: what pref?
16:44padenotehsan, it's a pref that lets us force enable a particular audio backend
16:44padenotehsan a new one
16:44ehsanbaku: also another question about the other bug... how do you feel about backporting this beast to branches? :(
16:44bakuehsan: I can. The issue for that patch is: what about if I'm on the main-thread and I need to retrieve the origin of a principal?
16:44padenotehsan, it's for power users and also to test our new rust pulse audio backend
16:44ehsanpadenot: do we need to know about it at startup?
16:45padenotehsan, yep, that's the issue
16:45padenotehsan, we do init this very very early because of sandboxing
16:45ehsanpadenot: life sucks :( r+ I guess
16:45ehsanthis whole content prefs setup is quite unsustainable
16:45bzpadenot: Please explain all this in teh commit message....
16:45bzer, the
16:46padenotbz, yes, will do
16:46bzThanks.
16:46bakuehsan: I mean: what about if i'm _not_ on the main-thread... for instance I/O thread and I want to retrieve the origin.
16:46bzehsan: well, we should be trying to drive this list to 0
16:46padenotbz, it's not
16:46bakuehsan: about porting that code: yes, I'm OK. it will take a bit more effort, because esr52 has DeviceStorage API yet.
16:46ehsanbaku: so yeah after I came up with that question, I saw that bkelly now has this use case also... so maybe it's time to stop resisting and just take your patch
16:47ehsanbaku: I don't really have any strong objections
16:47ehsanbz: it isn't doable, right?
16:47bkellyehsan: part of my problem with profiler is that if I want useful info I have to do a custom build to get full stacks AFAICT
16:47bakuehsan: cool.
16:47ehsanbkelly: false
16:47bkellyehsan: ok, how do I do it then?
16:47ehsanbaku: still waiting for an answer on the backport question :)
16:48bakuehsan: I answered: about porting that code: yes, I'm OK. it will take a bit more effort, because esr52 has DeviceStorage API yet.
16:48ehsanbkelly: it should be working on Nightly by downloading the extension from https://perf-html.io/
16:48bzehsan: well, not 0, but we can make it smaller
16:48ehsanbkelly: otherwise you're hitting P0 bugs
16:48* bz should actually make some progress on this...
16:48ehsanbz: well, the way we access prefs is a shit show
16:48bkellyehsan: oh, on nightly? I was trying dev-edition
16:48ehsanbkelly: on dev edition?
16:48ehsanI don't know...
16:49ehsandev edition in ancient :P
16:49ehsanbut it should work there as well I'd think
16:49bkellyehsan: I use my dev-edition as a clean profile I won't conflict with my main browsing process
16:49bkellyehsan: doesn't it just give you pseudo stacks by default?
16:50ehsanbaku: ok, we should probably exercise some serious caution here :) this patch makes me so nervous. the basic approach I like though
16:50ehsanmstange: is bkelly right in saying that Aurora doesn't let him walk full stacks?!
16:52ehsanbkelly: fwiw I really want the profiler to work out of the box on dev edition too, we have at least one person in the community who is really excited about perf testing on dev edition too
16:52ehsanI'd love to get their help in profiling
16:54bzehsan: fwiw, I still aim to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1343677
16:54firebotBug 1343677 NEW, nobody@mozilla.org Consider adding asserts in nsCSSProps::IsEnabled that we've received the prefs from the parent proce
16:54bzehsan: So we can maybe remove all the CSS stuff from that big list
16:55bkellyehsan: mstange: works much better on nightly in a fresh profile with fresh addon
16:55ehsanbz: didn't we already have an assert for that?
16:55bkellyehsan: search for nsHtml5LoadFlusher here: https://perfht.ml/2n6AQ6n
16:55bkellyehsan: in the child main thread
16:56ehsanbkelly: I do get the same thing but that seems like one runnable to me?
16:56bzehsan: We do, but the point is the stuff in the list is whitelisted from the assert
16:57bzehsan: And BoolVarCache is exempted from it
16:57ehsanbkelly: or at least not obviously multiple runnables
16:57bkellyehsan: yes... one runnable that takes longer than a frame
16:57ehsanbkelly: ah yes, that part is bs :)
16:57ehsanbkelly: I think I misread your email, I thought you meant more than one is being dispatched...
16:58bkellyehsan: thats my point... those runnables often take longer than a frame... we should give them a time budget and yield if necessary
16:58bzehsan: So if we try to use the cache before we have gotten prefs that won't trigger the assert, if I understand correctly
17:00ehsanbz: yes you're right...
17:00ehsanbz: I had thought about the pref cache issue before at least
17:00mstangeehsan: uh oh, I actually don't know what the status is on Aurora
17:00ehsanbkelly: so for one thing, we're doing a whole bunch of inefficient stuff here too
17:00ehsanmstange: that's unfortunate :(
17:00mstangeehsan: looks like https://bugzilla.mozilla.org/show_bug.cgi?id=1321065 made --enable-profiling the default for nightly but not for aurora
17:00firebotBug 1321065 FIXED, mh+mozilla@glandium.org Make --enable-profiling the default
17:01ehsanmstange: mconley will be sad about that :((
17:01mstange"return milestone.is_nightly"
17:01ehsanso will mystor
17:01ehsanand many more
17:01mstangeand myself
17:01mstangeso
17:01ehsanlet's queue up here
17:01mconleywat
17:01mstangethere's also https://bugzilla.mozilla.org/show_bug.cgi?id=1340721
17:01firebotBug 1340721 NEW, nobody@mozilla.org We're not taking advantage of frame pointers in non-Nightly Windows 32bit builds for stack walking i
17:01bkellyjust hung the browser trying to capture a profile washingtonpost load
17:02ehsanbkelly: :( got a link?
17:02mconleymaaaaaan
17:02bkellyehsan: I had to kill the browser
17:02ehsanjeez
17:02bkellyehsan: big spinner of death
17:02mystorehsan: What happened?
17:02ehsanthe home page?
17:02bkellyyea
17:02bkellyperhaps flash related?
17:03ehsanmystor: we don't have framepointers on aurora
17:03mystorehsan: Are frame pointers off outside of nightly or something?
17:03mstangebkelly: that hang might have been https://bugzilla.mozilla.org/show_bug.cgi?id=1346356 , which we left in the tree for three days to olong
17:03mystorugh that's gross
17:03ehsanpossibly anywhere besides nightly
17:03firebotBug 1346356 FIXED, n.nethercote@gmail.com Deadlock in the profiler because locked_profiler_stop notifies observers
17:03ehsansee backscroll
17:03* mystor just finished reading backscroll
17:03bkellyprobably
17:03mystorI am already sad that we don't have framepointers outside of nightly
17:03ehsanoh, good thing I haven't updated for a few days :D
17:04ehsanmstange: that sounds like the kind of bug we should respin nightlies for!
17:04ehsan(fwiw)
17:05mstangeehsan: probably; it was the weekend though so I thought we could keep the damage contained
17:05ehsanbkelly: I'll add some telemetry for this image loading thing to start getting a sense of what the situation looks like in the wild...
17:06mstangemystor: what do you need framepointers for on aurora?
17:06ehsanmstange: if you have the cycles please ping a sheriff on #devs, iirc spinning nightlies is simple through self serve
17:06bkellyehsan: wish we could collect stacks for any runnable taking more than 16ms thats not spending all the time in content js
17:06* ehsan needs to focus on finishing baku's review ;)
17:06mystormstange: ehsan wanted native stacks to be cheap to collect for bhr
17:06ehsanbkelly: mystor is working on doing that :) (for 100ms)
17:06mystorIIRC that requires frame pointers
17:07bkellyk
17:07mystorbkelly: Currently I'm waiting for an email reply from mreid
17:07mystorthe actual change isn't hard - the problem is how many gigabytes of data/day we're allowed to use
17:08mstangemystor: ok, so we have frame pointers on windows 32 bit builds now, and BHR uses them thanks to mconley's work
17:09mstangemystor: it's all the other platforms that don't have them yet; and the profiler doesn't attempt to use frame pointers for stack walking outside of nightly even if we have frame pointers
17:09mstange(because the profiler uses different stack walking code than BHR)
17:11mystormstange: OK, that makes sense
18:01* bholley bz: hm, it looks like ServoStyleSet::ResolveStyleForText doesn't call GetParent, at least directly: http://searchfox.org/mozilla-central/source/layout/style/ServoStyleSet.cpp#229
18:02bholleybz: and at least some of the callers get the parent style from the frame tree, rather than the sc tree (I haven't audited all of them)
18:06bzbholley: Right, it doesn't call GetParent
18:07bzbholley: It just takes parent styles in the form of an nsStyleContext
18:07bzbholley: And for first-line at least we will need something that takes them as a servo computed values or something...
18:07bholleybz: sure - but that can work even if mParent is bogus
18:07bholleybz: sure
18:07bholleybz: we can add an overload
18:07bzbholley: since there won't be a style context that represents the right thing
18:07bholleyright
18:07bzbholley: Fine. The GetParent() call is in first-letter
18:08bzbholley: which is trying to get the right parent style for the text
18:08bzbholley: so it needs an nsStyleContext, which it gets via GetParent() on itself.
18:08bzbholley: the GetParent() call I know of, that is.
18:08bholleybz: yeah, that'll need fixing - I'm just saying that the text resolution stuff isn't problematic in and of itself
18:09bzsure
18:13bkellywhat is an expanded principal used for?
18:14bzbkelly: extensions
18:14bzbkelly: e.g. webextensions
18:14bzbkelly: but in general, any time you need a principal that subsumes multiple codebases
18:14bzbkelly: it's only used as the principal of a sandbox
18:14bzbkelly: we have asserts, iirc, that it never ends up as the principal of a document.....
18:15bkellyhmm
18:29ehsan_git log
18:29ehsan_bah
19:44ehsan_mystor: https://docs.google.com/spreadsheets/d/1x_BWVlnQPg0DHbsrvPFX7g89lnFGa3lAIHWD_pLa_dE/edit#gid=626383860
20:10mystorWhat can the content process do right now with Preferences?
20:11bzget them
20:11bzif it's not too early
20:11mystorbz: It can't write prefs I assume?
20:11bzI _believe_ this is correct.
20:12mystormk, thanks
20:12bzPreferences::SetBool(const char* aPref, bool aValue)
20:12bz{
20:12bz ENSURE_MAIN_PROCESS("Cannot SetBool from content process:", aPref);
20:12bzhttp://searchfox.org/mozilla-central/source/modules/libpref/Preferences.cpp#56-62
20:45bkellyweird... my nightly session couldn't render the check boxes on gs.statcounter.com's chart editor... but restarting the browser fixed it
21:00smaugdoes anyone recall web pages which implement scrolling themselves (some other page than Google spreadsheet)?
21:00smaugI wonder if the latest nightly is a bit faster with such pages
15 Mar 2017
No messages
   
Last message: 162 days and 10 hours ago