mozilla :: #content

8 Aug 2017
01:23* bkelly tries to figure out how html streaming is supposed to work...
02:08* bkelly writes a bug...
13:53* bkelly tries a local asan build...
14:07* bkelly decides not to do an asan build...
14:07froydnjthat was fast
14:08bkellyfroydnj: well, I realized I can repro the leak with my existing optdebug build
14:09froydnjbkelly: \o/
14:18bz_sleepAnyone familiar with CC around?
14:18bkellyemail CC, yes
14:18bzheh
14:18bzMy real question is this: if we have a cycle containing JS things that we unlink
14:19bzAnd then someone exposes one of those JS things to script, so it effectively "becomes live"
14:19bzIs this a horrible problem or just a minor weirdness?
14:20bkellythis seems harder than email CC
14:20bzIt might be
14:20* bz is not sure
14:20bzemail CC can get pretty hairy, in its political dimensions. ;)
14:26bkellybz: in any case, both of these topics sound easier than the potty training I hear happening on the floor above
14:28* bkelly comments everything out until the leak goes away...
14:40* jgraham hopes that isn't related to the potty training
14:42bkellyno, but I do appreciate the symmetry of my life atm
14:52bzbkelly: All I can offer is that the potty training really shall pass. ;)
15:01bkellybz: the most frustrating thing about potty training for me is how they can be good at one day... and then lose interest and start having tons of accidents the next day
15:02bkellybz: can I make a cycle collected DOM class trace a jsapi member like JS:Heap<>?
15:14bzbkelly: That one is wrappercached, of course, but if you just take out the NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY and NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
15:14bzbkelly: And NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
15:14bzbkelly: then it should work for a non-wrappercached thing, I&#39;d thikn
15:14bzer, think
15:14bkellybz: what is the difference between trace and traverse?
15:15bztraverse is during the CC traversal: it follows strong-ref links to find cycles
15:15bztrace is during GC: it is used to mark things live if they&#39;re pointed to by a C++ thing
15:16bkellybz: so I trace js things and traverse c++ things?
15:16bzprecisely
15:16bkellyok
15:26bkellybz: unlink is for both c++ and js things, though, right?
15:29bkellyhmm, maybe not
15:37bkellywoah... this may be finally compiling
15:42bkellybz: can I send this cycle collection patch to you for review? its on the streams bug
15:43bzbkelly: sure
15:43bkellyit even fixes the memory leak!
15:43bzbkelly: I need to get to your other reviews on that... hopefully today
15:43bkellynp
15:43bkellybz: how long are you planning to be on vacation?
15:43bzbkelly: 2 weeks
15:44bkellyyour wife&#39;s university must start late? most places here are starting in a couple weeks
15:44bzbkelly: Might be working part of that second week; not sure yet
15:59smaugis it constructible or constructable or what?
16:00bzI&#39;d say &quot;constructible&quot;
16:00smaugok, thanks
16:00bzBut https://www.collinsdictionary.com/dictionary/english/constructable says that both spellings happen
16:10jgrahamWeirdly the OED suggests that neither exist in en-GB and only with an i in en-US
16:19mccr8bkelly: in bug 1128959 you need to call the trace method in your traverse, and clear mReader in Unlink
16:19firebothttps://bugzil.la/1128959 ASSIGNED, bkelly@mozilla.com Implement the WHATWG Streams spec
16:19bkellymccr8: ok... bz have you started or can I clear the flag?
16:20bzI have not started anything
16:20mccr8bkelly: oh wait maybe that&#39;s not a thing any more...
16:20bzCalling trace in traverse is not a thing anymore
16:21bkellyI saw some classes clearing in Unlink and some not
16:21bkellywasn&#39;t sure if it was necessary
16:21mccr8Right, I&#39;d forgotten. I don&#39;t remember if you need to clear in the unlink, though.
16:21mccr8We may auto do that now? Hmm.
16:21mccr8bkelly: I guess if it doesn&#39;t assert it is okay? Sorry for the confusion...
16:21bkellywell, it doesn&#39;t assert locally... but we will see what try says
16:22mccr8I think we just auto clear the JS pointers.
16:24bzhttps://bugzilla.mozilla.org/show_bug.cgi?id=1326507 was the thing that dropped the trace-from-traverse thing
16:24firebotBug 1326507 FIXED, bugs@pettay.fi What should happen when two classes that inherit from each other both want to trace JS things?
16:24bzmccr8: you reviewd it, even. ;)
16:25mccr8bz: yeah after I said that it came back to me... ;)
16:26bkellyok, I&#39;m just going to leave the patch as is
17:04bzmystor: ping
17:05mystorbz: pong
17:05bzmystor: so I don&#39;t do the mach bootstrap thing, long story
17:05bzmystor: What&#39;s the chance that the clang on my mac is new enough to Just Work
17:05mystorbz: How did you get this clang?
17:06mystorbz: If it&#39;s apple&#39;s clang, then no.
17:06bzApple&#39;s yes
17:06bzok
17:06mystorbz: If it&#39;s one downloaded from clang.llvm.org then it will worj
17:06mystorbz: (The big thing is just do you have llvm-config)
17:06* bz pokes
17:06bzoh
17:06bzI have that
17:07bz&#39;cause stylo
17:07mystorbz: The clang plugin also needs it (sorta like stylo), but for bad reasons doesn&#39;t use the LLVM_CONFIG environment variable
17:07mystorinstead it looks in the same directory as CC
17:07bzIf I just use the same clang I use for stylo, that should work?
17:07mystor(I think)
17:07mystorYeah, give it a show
17:07bzhrm....
17:07mystorshot*
17:07bzSo I have /opt/local/bin/llvm-config-mp-3.9
17:07bzWill it literally look for &quot;llvm-config&quot;?
17:07mystorI think there&#39;s a bug for unifying the llvm-config options
17:07* bz can symlink, of course
17:08bzAnyway, for now I can do this on try
17:08bzAnd avoid the hassle of local build. ;
17:08bzer, ;)
17:08mystorbz: Can you run: `clang++ -print-prog-name=llvm-config
17:09bz/opt/local/bin/clang++-mp-3.9 -print-prog-name=llvm-config
17:09mystorbz: It runs `clang++ -print-prog-name=llvm-config` if that fails it runs `which llvm-config` and if those both fail, then it errors out
17:09bzyes
17:09bzThat does work
17:09mystorawesome, then it should work
17:09bzOK, thanks
17:09mystor(If it errors out it will error out in configure so it&#39;s easy to test)
17:09bzwhere by `clang++ -print-prog-name=llvm-config` you mean $CXX -print-prog-name=llvm-config I hope
17:10mystoryeah
17:10bzgreat
17:10bzThanks!
17:10* bz goes back to poking things
17:10mystorbz: np
17:19smaughmm, where can I see UseCounter statistics
17:19smaugfroydnj: you might recall
17:19smaug(or implemented the stuff)
17:19froydnjsmaug: whereever we can get telemetry from
17:21smaugah, it is normal telemetry
18:06sheppyWe have an article for a non-standard <image> element. I presume it was a once-upon-a-time thing, either pre-specifications or from a proposed spec, but does anyone know where I can learn more about it so I can update the page to more accurately describe this situation?
18:13bzsheppy: looking
18:13bzsheppy: Whish article is this?
18:13sheppyIn particular, if we don&#39;t support it anymore at all, I want to say when it finally went away.
18:13bzsheppy: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/image ?
18:13sheppyhttps://developer.mozilla.org/en-US/docs/Web/HTML/Element/image yes
18:14bzSo the situation here is complicated
18:14bzAnd how to document it is unclear
18:14bzIf you do: var i = document.createElement(
18:14bzer, var i = document.createElement(&quot;image&quot;);
18:15bzthen you get an HTMLElement instance
18:15bzwith localName == &quot;image&quot;
18:15bzWith me so far?
18:16sheppyYeah
18:16bzOK. But now if your HTML says <image src=&quot;something&quot;>
18:16bzThen you get an HTMLImageElement instance
18:16bzwith localName == &quot;img&quot;
18:16sheppyThat&#39;s interesting. Why do we do that/
18:16bzBecause of the bits in https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inbody starting &#39;A start tag whose tag name is &quot;image&quot;&#39;
18:17bzWe do that because every browser has done it forever
18:17sheppyah-ha
18:17bzProbably going back to like the netscape 3 days....
18:17bzThe spec just reflects that
18:17sheppyYeah, that&#39;d be my guess. I was trying to find where it came from originally but looks like one of those lost-in-the-distant-past things.
18:17bzSo during parsing, <image> is an alias for <img> that makes the document not valid HTML
18:17bzOtherwise, &quot;image&quot; as a tag name is more or less like any other random tag name
18:18bzExcept you get HTMLElement instead of HTMLUnknownElement
18:18bzSo it&#39;s not _really_ a random tag name.....
18:18sheppyI like the text here. &quot;Parse error. Change the token&#39;s tag name to &quot;img&quot; and reprocess it. (Don&#39;t ask.)&quot; :)
18:19bzAnyway, I hope that helps...
18:20sheppybz: yes, very much, thank you
18:26mystorDoes anyone happen to know how we enable LTO in mozconfig?
18:46froydnjlto or pgo?
18:48* bkelly has found a downside to working from home.
18:48* bkelly is irrationally angry about hideous fence neighbors are putting in outside his window...
18:52digitaraldoverholt: TP foreground throttling; is it kicking in right when the page starts loading or is there a delay after page load?
18:52digitaraldI saw the pref, which seems to apply for backgrounded pages only
18:53overholtdigitarald: I think delay but now that you ask, I&#39;m not sure
18:53overholtdigitarald: email Ehsan?
18:53digitarald2nd question; would it be useful to have branches in the experiment, no/moderate/high throttling
18:53overholtdoubtful
18:53overholtdigitarald: the current prefs were tweaked over a while
18:54digitaraldtweaked against local tests I assume?
18:56overholtyes
18:59mystorfroydnj: Sorry, lto
18:59mystorfroydnj: (missed your msg)
19:00froydnjmystor: what platform is this?
19:01mystorfroydnj: I was wondering if there was something like ac_add_options - -enable-lto
19:01froydnjmystor: no
19:02mystorfroydnj: its more of a general &quot;how do we do yhis&quot; because I&#39;m under the impression we LTO shipped builds and don&#39;t do it for local ones but I might be off kilter
19:02mystor(I&#39;m mostly annoyed by how long stylo takes to LTO and I was thinking of making it so local builds don&#39;t have to LTO opt rust builds)
19:03froydnjmystor: amazingly, this is something that I am working on as we speak
19:03mystorfroydnj: ah, excellent. I was thing of doing it myself
19:04bzmystor: You can hack it locally pretty easily
19:04froydnjmystor: how were you planning on doing it?
19:04froydnjmystor: the current plan is to advertise a patch on dev-platform until the real solution (cargo fixes) arrives
19:04* bz won&#39;t speak to how to solve this in the general case
19:06mystorfroydnj: I was going to change Cargo.toml to say lto = false
19:06mystorfroydnj: And then add `-C lto` to RUSTFLAGS in the build system whenever ac_add_options --enable-rust-lto was turned on
19:06mystorfroydnj: Then modify the mozconfigs we use to build official builds to add that flag
19:06froydnjmystor: ah, so, rustc will yell at you about that
19:06mystorfroydnj: Whyyy?
19:07bzmm, story of compiling rust
19:07bz&quot;you will do it my way, or else. why would you want to test local changes?&quot;
19:07froydnjmystor: because -C lto is not valid for all rustc invocations (e.g. rlibs)
19:07mystorfroydnj: :&#39;(
19:07froydnjbz: yes =/
19:08mystorfroydnj: Ok - new plan - write a rustc wrapper which strips out -C lto arguments from the command line and set it as my rust prefix
19:08mystor:P
19:08froydnjmystor: https://github.com/rust-lang/cargo/issues/4349
19:08froydnjmystor: hah!
19:09froydnjmystor: this is not straightforward to do on windows
19:09mystorfroydnj: I can write it in rust?
19:09* mystor only really works on linux so it soothes the wound for me
19:09mystor*correction* only works on linux when they can get away with it
19:10froydnjmystor: I guess you could see if building a host rust program would be suitable
19:10froydnj&quot;I spend all my time building this rustc-wrapper&quot;
19:10mystorlol
19:10froydnj&quot;don&#39;t worry, you&#39;ll save time building xul&quot;
19:10mystoryeah, I was thinking of just doing it as a bandaid
19:10mystorI imagine it would build pretty fast
19:11froydnjthis is rust we&#39;re talking about :p
19:12mystorfroydnj: If you use a small enough amount of code rust can build in sub-1s
19:12bzLike &quot;main () {}&quot; ?
19:16mystorbz: heh
19:21mystorfroydnj/bz: I threw together the hackiest implementation of nolto ever :P
19:21mystorIt compiles in 0.4s, which is really bad for 10 lines of code, but *shrug*
19:22mystor(0.58s in --release)
19:32bkellysomething has changed and now I can&#39;t run firefox in vcxsrv on my windows machine any more
19:35dholbertstone: hi! I&#39;ve got an intern with some spare cycles at the end of his internship (he finished his main project and has another ~month) and I was thinking of having him take https://bugzilla.mozilla.org/show_bug.cgi?id=1361067 if you haven&#39;t started on it yet. You&#39;ve got a pending needinfo there RE whether you&#39;d like to take it after your current bug(s)
19:35dholbertbut it looks like you&#39;re not on it yet -- would it be OK if I stole it from you?
19:35firebotBug 1361067 NEW, nobody@mozilla.org coalesce mouse events to be once per refresh cycle (requestAnimationFrame, rAF)
19:39smaugdholbert: FWIW, it is probably best to implement that after bug 1351148
19:39firebothttps://bugzil.la/1351148 REOPENED, sshih@mozilla.com Add an event queue to nsThread for input events and annotate input IPC messages to use it
19:40dholbertsmaug: ah, ok
19:40dholbertmaybe best to leave it to stone after all then
19:40smaugdholbert: or just write the patch on top of bug 1351148
19:40* dholbert looks at that bug&#39;s patches
19:41smaugwe may manage to compress more mousemove events with bug 1351148
19:41smaugI mean in IPC layer
19:48dholbertsmaug / stone: never mind then -- I&#39;ve got another few bugs in mind for the intern, for the time being, and it seems like this particular bug wouldn&#39;t be the most efficient use of his time right now
19:48dholbertstone: so: forget that I said anything. :) take care!
19:50bzSo if I have an nsTArray
19:50bzAnd I want to iterate it starting at a given index
19:50bzI assume there&#39;s no nice range syntax for that?
19:51* bz sees no obvious way
20:08emiliobz: you can probably create a mozilla::Span, or however our array_view is called?
20:09emiliobz: i.e. for (const auto& foo : Span(aArray.Elements() + start, aArray.Length() - start))
20:10emiliobz: seems like that could be a helper method in nsTArray... that&#39;s quite ugly otherwise :)
20:12froydnjshould probably be able to do something like Span(aArray).From(start)
20:12froydnjbz: ^
20:19emiliofroydnj: ooh, nice
20:21emiliofroydnj: does it work for nsTArray though? It doesn&#39;t seem to from the header
20:22froydnjemilio: I guess that would be MakeSpan (nsTArray.h) or somehow using nsTArray&#39;s Span operator()
20:23emiliofroydnj: I guess, yeah... Nice :)
20:44bzok
20:45* bz turned out to not really want a ranged loop anyway, but will keep it in mind
20:47bkellyasuth: ping
20:47asuthbkelly: pong
20:48bkellyasuth: can you help me understand when this is called? http://searchfox.org/mozilla-central/source/netwerk/protocol/http/HttpChannelParentListener.cpp#317
20:48bkellyasuth: its in the parent side of e10s mode... but does it have any effect?
20:49asuthbkelly: checking notes
20:49bkellyI can trace it too... but I remember getting confused by this stuff before and I thought you might have some insights
20:57asuthbkelly: The parent only gets involved for redirects or when we give up. If you&#39;re looking at streaming, you shouldn&#39;t have to worry about it.
20:57asuthwhich is to say, what you already suspected
20:57bkellyasuth: ok... I am splitting &quot;start&quot; from &quot;finish&quot;... so I will just do start+finish at the same time there
20:58bkellyasuth: thanks for looking!
20:58asuthbkelly: that sounds like a good way to do it
20:58bkellyasuth: btw, I am throwing you under the review bus on these patches :-)
20:58asuthbkelly: I was hoping I had some prettier diagrams that covered this case, but the redirect dance is one of the deeper nightmares I ran into while trying to explain what was happening
20:58asuthbkelly: yes, I agree I probably am a suitable reviewer for this case
20:59asuthbkelly: and hopefully there should be synergy with that then leading into me landing the parent intercept changes on its heels
20:59asuthone week before 57 merges to beta! ;)
21:01bkellyasuth: btw, if putting it behind a pref like I asked that one time is too much, feel free to say so
21:04asuthbkelly: Will do. The spot where it ended up on the shelf was me attempting to revisit your performance investigations in the face of the enhancements to HttpChannel{Parent,Child} moving their data transport off of the main threads and PContent and other main-thread cleanup work. In particular, I wanted to see where we took the latency hits, and if that
21:04asuthspecific spot would be something that would be easier to add a pref to avoid.
21:05asuthBecause it seemed like there was a lot of cleanup in parent intercept that could still be a win.
21:05asuthMy hybrid idea was to create the parent actors regardless.
21:05asuthAnd in the prefed &quot;still be child intercept&quot;, we&#39;d end up killing them off once we were done.
21:05asuthOr rather, if we didn&#39;t need them.
21:06asuthAnd/or they just wouldn&#39;t be in the critical path, because we&#39;d keep the bypass in the child.
21:08bkellyyea, that would probably be fine
21:08asuthbkelly: In particular, while you&#39;re doing your stream enhancements, it&#39;d be interesting to see if we can do tricks to use our new IPCBlob/IPCStream magic so that we can avoid pumping the data up to the parent to pump it back down.
21:09asuthThe bypass of course matters less when serving out of cache, but if the streams API catches on, that will start to be a bigger concern.
21:09asuther, serving out of Cache API.
21:13bkellyasuth: I guess I would have to see what you are thinking, but we might want to wait on those kinds of optimizations
21:19asuthbkelly: Agreed on the waiting, I mean more like I&#39;d love it if you left a brainstorm brain-dump on the bug on any thoughts you have about how the optimizations would work and/or how far we are from them right now.
21:20asuthThe http channel code in particular is very depressing; seeing any lights at the end of the many tunnels is always nice.
21:21bkellyasuth: so, I thought a lot when writing cache API on how to avoid bouncing data around like this... and I concluded trying to optimize it away is more trouble than its worth at the moment
21:21bkellyI didn&#39;t have a good solution
21:22bkellyand its unclear if the benefit would be that great
21:22mrbkapDo we trust C++ compilers & RVO enough these days to return ns[C]Strings directly instead of through an outparam?
21:22asuthfair enough
21:27bkellyasuth: the other issue I had when trying to solve this for cache API was that you could end up buffering data in the parent which was bad.... we&#39;d rather have memory usage in the child process
21:30asuthbkelly: eh, okay yeah, I&#39;ll stop worrying and assume that in the future the IPC endpoint stuff or just handing pipe FD&#39;s between sibling processes is possible and we can bypass the parent with full efficiency when it shows up on profiles or other browsers are mo faster.
21:31bkellyasuth: I think it could all change as we add back pressure as well
21:31bkellywe&#39;ll have to see how it plays out
21:41billmbz: what&#39;s hard about debugging pageload in e10s?
21:44smaughmm, what code might print &quot;serialization: Arial&quot; to terminal
21:44smaugstylo? (this is a stylo build)
21:45mccr8smaug: it is already fixed on m-c
21:45mccr8stylo debug printf
21:46smaugaha. This was very recent m-i
21:47mccr8I don&#39;t know where the fix actually is, but they landed it somewhere.
21:47mccr8The spam is also in Nightly
21:48bzbillm: How do I do it?
21:49billmbz: I guess I would attach to the process and load the page
21:49bzbillm: So the way I would do it in non-e10s is open a new tab, paste my url into the url bar, set the relevant breakpoints, hit enter
21:49bzbillm: attach to which process? We create the process when enter is hit in the url bar
21:49billmbz: so you specifically care about the case where a new process is started?
21:49billmbz: usually I just start at about:home
21:49bzno, but I specifically care about the case when the session history is empty
21:50billmbz: ok, that&#39;s harder I guess
21:50bzYes, yes it is
21:51billmbz: MOZ_DEBUG_CHILD_PROCESS works for that, but it&#39;s much less user friendly
21:52billmbz: also, I guess with one content process, you know which process to attach to
21:52bzIf I&#39;ve already loaded some content page, yes
21:53bz(kinda; I still have to hunt it down)
21:53bzBasically, the experience is way worse than --debugger=lldb and then just opening a non-e10s window. :(
21:53billmbz: yeah, I wish we showed the pid in the tab tooltip even when processCount=1
21:53bzWhich is slightly unavoidable....
21:54billmbz: yeah, I don&#39;t deny that it&#39;s worse. not sure about rocket science though :-).
21:55billmbz: a long time ago I had it set up so that MOZ_DEBUG_CHILD_PROCESS would open a new terminal with a debugger attached to each process that started
21:56billmit didn&#39;t work on mac, though
21:56billmand it was slow
22:00jdmthat sounds slick
22:40* bz wonders what the right procedure is to add a wpt reftest
22:57smaugwhy I&#39;m getting way lower speedometer scores with stylo-builds (stylo enabled)
23:06bzjgraham: ping
23:06bzsmaug: https://bugzilla.mozilla.org/show_bug.cgi?id=1371496 ?
23:06firebotBug 1371496 NEW, bobbyholley@gmail.com stylo: Too much time spent styling on Speedometer Inferno benchmark
23:07cynicaldevilbz: Could I ask you some questions about Gecko&#39;s parser code?
23:07bzcynicaldevil: can try, but I have to get dinner soon
23:08cynicaldevilbz: Just a single question then, do you know what&#39;s the use of intended parent in this function: https://dxr.mozilla.org/mozilla-central/source/parser/html/nsHtml5TreeBuilderCppSupplement.h?q=%2Bfunction%3A%22nsHtml5TreeBuilder%3A%3AcreateElement%28int32_t%2C+nsIAtom+%2A%2C+nsHtml5HtmlAttributes+%2A%2C+nsIContentHandle+%2A%2C+nsIContentHandle+%2A%29%22&redirect_type=single#348 ?
23:09bzcynicaldevil: no, sorry
23:09* bz would have to dig to figure it out
23:09cynicaldevilbz: It&#39;s alright, I&#39;ll do the digging :)
23:16smaugcynicaldevil: you&#39;ve looked at https://bugzilla.mozilla.org/show_bug.cgi?id=1091425 ?
23:16firebotBug 1091425 FIXED, william@fastmail.com Create elements in the document on the &quot;intended parent&quot;
23:17cynicaldevilsmaug: I haven&#39;t, thanks for the link!
9 Aug 2017
No messages
   
Last message: 13 days and 21 hours ago