mozilla :: #servo

7 Aug 2017
06:18Manishearthheycam: [a,b,c].iter()?
06:18Manishearthyou also have once() and repeat() for similar one-off iterator sources
06:18heycamManishearth: ah nice, yes!
06:19heycamthanks
06:19heycamand now to pretend like my function returns () so I can find out what the real return type should be :-)
06:27bholleynjn: I'm here for a few minutes before heading to bed if you want to sync up
06:31bholleyheycam: xidorn: do either of you have cycles to work with njn on bug 1367854 and figure out where our 10% deficit on AWSY is coming from?
06:31firebothttps://bugzil.la/1367854 NEW, nobody@mozilla.org stylo: Memory usage seems to be higher than Gecko
06:32xidornbholley: I can probably try. I don't think I have much to do in my plate
06:33bholleyxidorn: ok great, thanks
06:33xidorn(although I'm not sure what to expect there either)
06:33heycambholley: was going to say I could look later in the week, but xidorn can have it :-)
06:33xidornheycam: I don't have much experience on working on memory things, so I may need help at some point :)
06:34heycamxidorn: sure, I'll be around
06:34bholleyxidorn: njn should also be able to help
06:34bholleyxidorn: the first question is whether the memory footprint is coming from stuff we expect (ComputedValues, stylist hashtables, parsed stylesheets), or something else
06:34xidornI guess I need to do a dmd build first
06:35bholleyxidorn: njn's measurements in bug 1367854 more or less show stuff we expect
06:35xidornbholley: what do we expect would be larger?
06:35bholleyxidorn: nothing we expect to be larger per se, just things we expect to be big
06:35xidornbholley: oh, okay
06:35bholleyxidorn: the thing that confuses me is that bug 1368290 didn't make much a dent in the graph
06:35firebothttps://bugzil.la/1368290 FIXED, emilio+bugs@crisal.io stylo: Implement ComputedValues sharing for text and anonymous boxes
06:36bholleyxidorn: since that bug significantly reduced the number of ComputedValues
06:36bholleyxidorn: unless we still have bad sharing on some of the sites on AWSY
06:38bholleyxidorn: this commit does logging on the number of nsStyleContexts in the system: https://github.com/bholley/gecko/commit/228a28ce1d0b62eeb23224b727ab55551f5a0e57
06:39bholleyxidorn: it's what I used to generate the numbers in bug 1367862 and bug 1369902
06:39firebothttps://bugzil.la/1367862 ASSIGNED, bzbarsky stylo: Not much ComputedValues sharing going on on the Obama wikipedia page
06:39firebothttps://bugzil.la/1369902 NEW, bzbarsky stylo: Not much ComputedValues sharing going on on github diff page
06:39bholleyxidorn: bug 1368291 will help the numbers some, but probably not as much as text/anonbox sharing did, so it seems unlikely that it would make a bigger difference in the graph
06:39firebothttps://bugzil.la/1368291 NEW, bobbyholley@gmail.com stylo: Enable ComputedValues sharing for lazy pseudos
06:39bholleyxidorn: (either way I'll try to get that landed tomorrow)
06:40bholleyxidorn: so it basically boils down to the stylo-vs-gecko memory usage comparison of nsStyleContexts, parsed stylesheets, and selectormaps
06:41xidornbholley: lazy pseudos would have their style context created only when requested during frame construction or layout, is that correct?
06:41bholleyxidorn: correct
06:41bholleyxidorn: same for anon boxes / text, FWIW
06:42bholleyxidorn: so we want to know if any of those three things above consume enough extra memory in stylo to account for the 10% difference, and if not, then what
06:43xidornbholley: what's the story of sharing between threads?
06:43xidornbholley: I think we don't share in that case, do we?
06:43bholleyxidorn: we do share, but it's per-thread
06:43bholleyxidorn: so sharing is somewhat worse
06:43bholleyxidorn: but I'm happy just focusing on STYLO_THREADS=1 for now
06:43xidornbholley: so we even regress 10% for STYLO_THREADS=1?
06:43bholleyxidorn: yes
06:43bholleyxidorn: https://treeherder.mozilla.org/perf.html#/graphs?timerange=604800&series=%5Bmozilla-inbound,a630e0d4f7000610ca57d0f8da52b55d117632a9,1,4%5D&series=%5Bmozilla-inbound,eb19722dabafa2ad2c817df4f2b650a13b3ce984,1,4%5D&series=%5Bmozilla-inbound,5846d174e9e152bc853b8761026dd7b2d762b572,1,4%5D&series=%5Bautoland,5846d174e9e152bc853b8761026dd7b2d762b572,1,4%5D&series=%5Bautoland,eb19722dabafa2ad2c817df4f2b650a13b3ce984,1,4%5D
06:44bholleyxidorn: parallel is a bit worse than sequential, but the difference between any stylo and gecko is much larger
06:44xidornbholley: do we share style structs between style contexts?
06:45bholleyxidorn: yes
06:45bholleyxidorn: it's copy-on-write
06:45xidornoh, right
06:45bholleyxidorn: when we cascade an element, we allocate a new ServoStyleContext
06:46bholleyxidorn: and then start out with pointers to the parent/initial style structs
06:46bholleyxidorn: and then any mutation causes us to make a copy, otherwise we share
06:58xidornleaving for gym now. will have a closer look when back
07:27bholleyxidorn: thanks!
07:28* bholley -> bed
07:38Manishearthoh hey the nightly new tab page does DDG search
07:38Manishearthawesomebar does not
07:38Manishearthinteresting compromise
07:41Manishearthugh but it broke ctrl-tab-type-url
07:42est31whats that
07:42est31I only know of
07:42est31ctrl-t-type-url
07:43est31and ctrl-l-type-url
07:44Manishearther, ctrl-t is what i mean
07:51fwiwManishearth: isn't that depend on default search provider?
07:56est31Manishearth: I'm still a bit sad that about:newtab cant be overridden any more
07:56est31always have set it to about:blank
08:09Caspy7est31: I don't have a reference atm, but was thinking there was going to be a WE API
08:10fwiwWE API?
08:10Caspy7webextension
08:11Caspy7sorry, probably shouldn't use that, picked it up from some others...
08:12Caspy7wait, we're in #servo and I'm half asleep, maybe this was not a Firefox reference and I entered into the middle of a convo...
08:14fwiw:)
08:16Manishearthest31: sure it can. the whimsypro extension overrides it
08:16emilioheycam: around?
08:16heycamemilio: hi!
08:16Manishearthest31: https://github.com/bwinton/whimsy/tree/webextension
08:16emilioheycam: I noticed you self-assigned the "preserve per-stylesheet-origin stuff" in the stylist
08:17heycamemilio: don't tell me, you have patches already?
08:17emilioheycam: nope, but since I'm looking at bug 1386045 and stuff might interleave a bit, I was curious about your approach. I have some WIP stuff, but just for testing it, so not really anything close to landable yet
08:17firebothttps://bugzil.la/1386045 NEW, emilio+bugs@crisal.io stylo: Lots of time spent refreshing the stylist on facebook.com
08:18heycamemilio: ah ok. so the approach I'm starting with is to invert the way we store the selector maps, so keyed off origin first, and move the selector maps and invalidations and other stuff into a struct that represents the cascade data for a given origin
08:19heycamemilio: so, not solving anything to do with cross document sharing yet
08:19emilioheycam: heh, ok... I had the "keyed-off-origin first" for the PerPseudoElementSelectorMap stuff basically done. Want me to land that before-hand?
08:19heycamemilio: heh I've also got that patch already written :-)
08:19emilioheycam: yeah, sure... I'm a bit unsure cross-doc sharing is worth the effort right now
08:20heycamemilio: let me push what I've got so far and you can tell me if there are better patches in your queue
08:20emilioheycam: oh, ok, then go ahead. If you point me to it, I can base my stuff on yours instead
08:20* emilio is glad he pinged heycam :P
08:20Manishearthheycam: btw lmk if you have bugs for me to work on
08:21heycamemilio: I was thinking of pinging you when I started looking this morning but had to wait until you were awake anyway ;-)
08:21Manishearthi planned on doing some triage and mochitest/reftest chasing, but if there are known priority bugs i can work on those
08:21emilioheycam: heh, fair enough :P
08:21heycamemilio: do you know if there's a good way to push git-cinnabar based things to a fork in my git repo..
08:21* heycam will just push the patches to the bug otherwise
08:22heycam*fork in my github account
08:22emilioheycam: not really... It'll basically take forever aiui
08:22heycamemilio: yeah, pushing everything
08:22* heycam pushes to the bug
08:25emilioheycam: If those are landable, I'm happy to review a PR for those. specially the first one
08:25heycamemilio: sure, happy to get that in first
08:25heycamemilio: let me do a try run then I'll open a PR
08:26heycamoh I did a try run for the first patch already
08:27emilioheycam: nice!
08:28emilioheycam: my first improvement for the rebuild times was to avoid branching on the origin in the inner per-selector loop, so that is needed preliminar work for that
08:28heycamemilio: ah, interesting
08:32emilioheycam: r=me % nits
08:32heycamemilio: thanks!
08:49emilioheycam: well, your patch made it pretty trivial :-). r? ^
08:50heycamr+
08:51emilioheycam: thanks :)
08:52heycam(well I guess homu won't be happy with the other commit under it, so maybe I shouldn't have r+ed directly)
08:53emilioheycam: well... Yours should merge first, so I expect homu to be able to figure it out
08:55heycamemilio: I did do a nit fixup though, might confuse it
08:56* heycam shrugs
08:56emiliooh well
09:03emilioheycam: how crazy does it sound to move DynamicAtom to a public header, so we can avoid the virtual call in Gecko_AddRefAtom/Gecko_ReleaseAtom?
09:04emilioheycam: it shows in profiles fairly frequently
09:04* emilio maybe should discuss it with froydnj
09:22heycamemilio: maybe sounds good, but yes I'm not the person to ask :-)
10:41cynicaldevil_nox: ping
10:44emiliocynicaldevil: he's on pto
10:45emilioSimonSapin: r? ^
10:45cynicaldevilemilio: I thought he was back today? He told me it was for two weeks...
10:45emiliocynicaldevil: if I can help, just ask :)
10:45emiliocynicaldevil: oh, maybe, idk
10:46* boris is waiting for nox's review~~
10:46emiliocynicaldevil: I said that based on that he's still _nox and not nox...
10:46_noxAm becoming public again very soon once I'm sat in my train.
10:47_noxboris: You should have asked for someone else to review the thing though, I'm not irreplaceable.
10:48boris_nox: I think you are the most suitable person to review these patches. it's ok to wait for few days.
10:51cynicaldevilemilio: I'm looking for a case where this condition: https://github.com/servo/html5ever/blob/master/html5ever/src/tree_builder/actions.rs#L810 fails, where the insertion point is not in the same tree as the form element.
10:54travis-ciServo failed to build with Rust nightly: https://travis-ci.org/servo/servo-with-rust-nightly/builds/261776356 CC nox, SimonSapin
10:56emiliocynicaldevil: no clue, sorry... Given the implementations of that function in servo, I'm not sure which APIs could trigger that... Maybe make a try run with them returning true unconditionally, and see what fails?
10:57_noxShould be done with reading my ton of emails when I sit down in the train.
11:02jgrahamYOu only got 100 emails?
11:04cynicaldevilemilio: Hmm, I'd thought of that too, I'll suggest it to jdm.
11:05cynicaldeviljgraham: a ton means 1000 where I'm from.
11:11jgrahamIt's ~1000kg, or a unit of capacity equal to 100 cubic feet. It's also (colloquially) used to refer to doing 100mph particularly on the motorway. I don't know if _nox measures his email by mass volume or velocity.
11:11SimonSapinemilio: replied, the commit/PR message could use some more motivation
11:27emilioSimonSapin: fair enough, just replied inline, will update the commit message :)
11:31SimonSapinemilio: I dont understand. Hasher.write_u32(self.get_hash()) still hashes the hash
11:31emilioSimonSapin: the point is that I'm going to introduce a hasher that basically does nothing, and expects a single u32 as the input
11:31emilioSimonSapin: that already holds for all the Gecko atoms, but not the Servo ones
11:31SimonSapinah, thats the context I was missing
11:32SimonSapinemilio: and that should be in the commit message
11:32SimonSapinr+ with that
11:33emilioSimonSapin: just did that
11:37emilioSimonSapin: thanks!
11:42SimonSapinemilio: isnt your no-op hasher independent of fnv vs fxhash?
11:46emilioSimonSapin: right, but I'm changing most of the hashmaps to use the noop hash
11:46emilioSimonSapin: r? ^^ btw
11:46emilioSimonSapin: I'll look at which ones remain, but I suspect those are the per-pseudo hashmaps, which I also want to get rid of
11:47emilioSimonSapin: and maybe the animation ones or what not
11:47SimonSapinemilio: Id prefer if PrecomputedHasher contained Option<u32> instead and panicked when used incorrectly. What do you think?
11:47emilioSimonSapin: sounds good to me
11:50emilioSimonSapin: just did that :)
11:52SimonSapinemilio: also assert!(self.hash.is_none()) in write_u32?
11:55xidornSimonSapin: any suggestion for the crate that truncates the output from dtoa?
11:56emilioSimonSapin: fair, done
11:56xidornSimonSapin: also, if I would be creating that crate, should I put it in my account, or under servo org?
11:59emilioxidorn: probably any of both are fine
11:59SimonSapinxidorn: for the name? Lets see the key point is that it rounds to a number of significant digits rather than a number of decimal places, right?
11:59xidornSimonSapin: yeah
11:59SimonSapinxidorn: Id say in the org in case we need to do something while youre away
12:00* _nox is in.
12:00SimonSapinalso run &quot;cargo owner -a github:servo:cargo-publish&quot; and with a few other people after the first upload on crates.io
12:00xidornSimonSapin: ok. I guess there would need some config work...
12:01* SimonSapin peeks next door
12:01SimonSapinnox: no youre not
12:01noxIn the train.
12:01SimonSapinxidorn: you mean for bors/homu? Thatd require config regardless of org
12:02xidornSimonSapin: yep, those
12:02xidornanyway... name is the most important one :/
12:02xidornI don&#39;t have a good sense of naming something...
12:02SimonSapinno one does
12:02SimonSapinsignificant-dtoa?
12:03SimonSapinalthough d in dtoa is double, right?
12:03SimonSapinsignificant-float-fmt
12:04noxcynicaldevil: What do you want me to look at?
12:04noxboris: Same.
12:04xidornSimonSapin: significant actually sounds weird here :/
12:04SimonSapinxidorn: I dont know how else to describe what it does
12:05xidornsimple-float-fmt?
12:05SimonSapinwhat makes this simple?
12:05xidornno options?
12:05xidornlike... it can only do one thing...
12:05SimonSapinbut what thing that is is more important than configurability
12:05cynicaldevilnox: I was trying to come up with a case which fails this condition: https://github.com/servo/html5ever/blob/master/html5ever/src/tree_builder/actions.rs#L810
12:06cynicaldevilHow can it be that the insertion algorithm returns a point which is not in the same tree?
12:06xidornSimonSapin: web-float-fmt? :)
12:06xidornor css-float-fmt :/
12:06noxcynicaldevil: If the form has been removed by a script meanwhile.
12:07SimonSapinits not really related to the web or css
12:07xidornwell... yeah...
12:07cynicaldevilnox: good point, but then this: https://github.com/servo/html5ever/blob/master/html5ever/src/tree_builder/actions.rs#L804 would return false, and control would never reach same_tree in the first place.
12:08noxcynicaldevil: Why?
12:08noxThe parser doesn&#39;t know that the form element has been removed from the tree.
12:08cynicaldevilnox: That&#39;s a good point too.
12:10cynicaldevilnox: Right now, same_tree accesses the DOM to return the result, so doesn&#39;t that cause a problem when the script is manipulating the DOM as well?
12:12xidornI guess I need to go to bed now... so sleepy...
12:13noxcynicaldevil: I&#39;m not sure I understand what you mean.
12:14emiliocynicaldevil: the parser doesn&#39;t run concurrently w/ script, right?
12:14cynicaldevilnox: nvm, got confused
12:25SimonSapinnox: r? https://github.com/servo/servo/pull/17994
12:25crowbotPR #17994: Update geckolib Rust version to 1.19.0 - https://github.com/servo/servo/pull/17994
12:25SimonSapin(or anyone)
12:27emilioSimonSapin: r=me
13:07est31nice
13:07est31so this is the policy that firefox ended up with
13:07est31https://wiki.mozilla.org/Rust_Update_Policy_for_Firefox
13:07est31cool
13:10karyonhi! i&#39;m interested in the &quot;Senior Research Engineer - Servo&quot; job posting and wanted to survey the situation before sending a full-blown application. has anybody got time for a chat?
13:40emilioTYLin: ping?
13:40SimonSapinkaryon: in general, just ask questions and stick around, someone might answer a bit later
13:49karyonokay, thanks. some of my questions are, where the servo team members are located currently and in how far the &quot;senior&quot; is a strict requirement - i have an MA in computer science but that&#39;s it. the job posting itself doesn&#39;t mention any specific requirements in that regard.
13:55SimonSapinkaryon: were very distributed geographically, almost everyone in a different city
13:56SimonSapinmetajack would be the one to ask about requirements, hes not online right now
13:58borisemilio: I guess TYLin is preparing his luggage to North Korea
13:58emilioboris: huh, how so?
13:59borisemilio: and he will be offline this week because there is no internet in North Korea
13:59emilioboris: well, he ni?d me 10 minutes ago in a bug, but sure, no problem, thanks! :)
13:59jackIf in doubt you can apply anyway. Senior usually comes with a few years of work experience. Some characteristics (but not only ones) are able to self direct and mentoring others. People reach that point at different speeds and in different ways
14:00jackBut I&#39;m on pto so email me if you have more questions and I&#39;ll get to it ina few days
14:00borishaha, maybe he will reply you before he go to sleep
14:04karyonSimonSapin, jack: okay, thank you both very much. i&#39;ll most definitely apply :)
14:05SimonSapinah, I forgot jacks nickname is not metajack on IRC
14:25emilioSimonSapin: r? ^
14:41SimonSapinemilio: commented
14:42emilioSimonSapin: good call, I&#39;ll measure it and come back with numbers
16:32emilioSimonSapin: more easy-r?s ^
17:13pcwaltonstandups: Starting on a WebGL-based demo for Pathfinder 2 for broader platform testing and as a testbed for the GLSL.
17:13standupsOk, submitted #49221 for https://www.standu.ps/user/pcwalton/
17:17ajeffreypcwalton: review ping for #17862?
17:17crowbotPR #17862: Implemented paint worklets drawing to a border. - https://github.com/servo/servo/pull/17862
17:17ajeffreygw: review ping for #17845?
17:17crowbotPR #17845: Use CSS background-size property when computing the size of a paint worklet - https://github.com/servo/servo/pull/17845
17:18ajeffrey(ah summer holidays, the season for review pings :)
17:28pcwaltonajeffrey: on it
17:29ajeffreypcwalton: thanks!
17:30mbrubeckpcwalton: In the parallel layout simplification we talked about at the all-hands,
17:31mbrubeckpcwalton: within a block formatting context, layout of block-level boxes would be sequential always? or only if the context includes floats?
17:35pcwaltonmbrubeck: so the idea was to make it parallel once we dynamically discover there are no floats in any subsequent subtrees
17:36mbrubeckpcwalton: Okay, makes sense.
17:36pcwaltonso its like for each kid (sequentially): lay out kid sequentially; if no floats out of that kid AND no subsequent kids have floats in their subtrees then break; end for; for each remaining kid (parallel): lay out
18:06avadacatavracrowbot: tell jdm maybe this is the android problem: https://github.com/android-ndk/ndk/issues/215
18:06crowbotyou got it!
18:06crowbotIssue #215: stddef.h: No such file or directory - https://github.com/android-ndk/ndk/issues/215
18:37cynicaldevilnox: Would something like this be enough for the condition I mentioned earlier to fail? https://www.irccloud.com/pastebin/lkPvzUxt/
18:48pcwaltontime to relearn modern JS app development
18:49noxcynicaldevil: I think so.
18:50cynicaldevilnox: Doesn&#39;t work. same_tree still returns true.
18:50cynicaldevilShould I open a PR to perform a try run?
18:52noxThat&#39;s weird.
18:54noxcynicaldevil: Did you log that it returned false?
18:54cynicaldevilnox: You mean did I add any debug statements should it return false?
18:54cynicaldevilyes
18:55noxOk.
18:56noxcynicaldevil: Oh I see, I think.
18:57noxcynicaldevil: I don&#39;t think h5e is doing the check correctly.
18:57noxcynicaldevil: &quot;the intended parent is in the same tree as the element pointed to by the form element pointer&quot;
18:57noxWait no, I misread, we do that correctly.
18:59cynicaldevilnox: Maybe the fault is in &#39;is_in_same_home_subtree&#39; method.
19:01cynicaldevilnox: If we remove the node from the tree, then the node&#39;s root element must be the node itself, right?
19:01noxYes
19:01noxOh
19:12cynicaldevilOoh interesting.
19:13cynicaldevilnox: I just added some more debug statements in the root_element method, and found that both, the insertion point and the form won&#39;t be in the doc.
19:18cynicaldevilnox: I know why same_tree is returning true here now. the insertion point IS the form element.
19:20noxcynicaldevil: Oh right.
19:20noxcynicaldevil: Do <form><div><script>/* remove the div */</script><input>
19:22cynicaldevilnox: that does the trick.
19:23cynicaldevilnox: Should I open a PR to perform a try run anyway?
19:24cynicaldevilto see what kinds of tests fail
19:24noxcynicaldevil: Yeah that would be nice, to compare with Gecko.
19:30bz_awayAcross all elements that match that rule
19:30bz_awayer, wrong window
19:40cynicaldevil18000!
19:41cbrewsterlucky number
19:47mbrubeckpcwalton: Do you know why the `ComputeAbsolutePositions` traversal isn&#39;t parallel in Servo layout? Is it just not worth the overhead?
19:49pcwaltonmbrubeck: yeah, its not worth the overhead when I measured
19:50cbrewsterDoes servo support `-webkit-filter`?
19:50pcwaltonit supports filter:
19:50cbrewsterbut no webkit prefix? (Some custom element tests are failing that use it, just want to make sure its not an issue with custom elements)
19:52mbrubeckcbrewster: in style/properties/longhands/effects.mako.rs, it has extra_prefixes=&quot;webkit&quot;
19:53cbrewstermbrubeck: hmm :/ thanks
19:55cbrewsterahh CSSStyleDeclaration.webidl does not have attributes for any webkit prefixed properties.
20:22cynicaldevilnox: This is the first test which fails: https://github.com/servo/servo/blob/master/tests/wpt/web-platform-tests/html/semantics/forms/form-control-infrastructure/form_owner_and_table_2.html
20:23cynicaldevilI wonder how it passes through Gecko
20:24cynicaldevilmaybe it does these checks on the main thread itself, while executing the tree ops.
20:51Manishearthstandups: remove style/testing (pull 17984)
20:51standupsOk, submitted #49234 for https://www.standu.ps/user/Manishearth/
20:51Manishearthstandups: quick crashfix (bug 1387953)
20:51standupsOk, submitted #49235 for https://www.standu.ps/user/Manishearth/
20:53Manishearthjyc: ping bug 1385439
20:53firebothttps://bugzil.la/1385439 NEW, jyc@eqv.io Absolute calcs in line-height do not get zoomed in text-zoom
20:53Manishearthfroydnj: there?
20:53jychi manish!
20:54froydnjManishearth: sort of, yes
20:54Manishearthbholley: got any other things for me to work on?
20:54Manishearthfroydnj: need help with the stylo-tests stuff? I just queued a patch for removing the &quot;testing&quot; feature entirely which might simplify things for you
20:54jycoh, Manishearth: still need a way to test that, asked Daniel & he suggested using a mochitest. was planning on tackling that after getting some animations stuff to work with properties & values
20:54Manishearthjyc: cool
20:55Manishearthjyc: yeah it&#39;s not an urgent bug, just making sure you didn&#39;t make progress and forget to post it there or something :)
20:55jycManishearth: haha cool! yep makes sense
20:55jycManishearth: yeah, would be cool to find out why setting that pref crashes too
20:55froydnjManishearth: if you could review #17925, that would be helpful
20:55crowbotPR #17925: move stylo_test build script guts from Python to Rust - https://github.com/servo/servo/pull/17925
20:55Manishearthfroydnj: ohhh right
20:55Manishearthi thought i finished that but I didn&#39;t
20:56Manishearthfroydnj: IIRC the behavior seemed correct, but let me do a final look through
20:56froydnjManishearth: I&#39;ll also be posting a pull request to add features to stylo_tests so we can propagate things like bindgen and whatnot down to its copy of the style crate
20:56Manishearthfroydnj: doesn&#39;t that work already?
20:56Manishearthif you set the MOZ_OBJECTDIR or whatever it works
20:57bz_awaycbrewster: note that Gecko auto-generates those bits of CSSStyleDeclaration.webidl (or its equivalent)
20:57bz_awaycbrewster: So it can&#39;t get out of sync with the supported prop list
20:57Manishearthbut yeah we definitely don&#39;t want to run bindgen twice
20:57bz_awaycbrewster: And gets all the right -webkit bits, etc
20:57Manishearthfroydnj: sorry about the delay on that
20:57froydnjManishearth: does it build the style for stylo_tests with the features that gkrust builds style with if you don&#39;t have anything special in stylo_tests&#39;s Cargo.toml?
20:57Manishearthfroydnj: feel free to bug me incessantly about reviews or questions wrt this :)
20:57froydnjthat doesn&#39;t match my mental model, but maybe my mental model is wrong!
20:58Manishearthfroydnj: oh. not by default, no
20:58Manishearthat least i don&#39;t think so!
20:58Manishearthwell
20:58Manishearthhm
20:58Manishearthmaybe
20:58Manishearthlet me just check
20:58froydnjManishearth: that&#39;s what I thought too!
20:58Manishearth[dev-dependencies]
20:58Manishearthstylo_tests = {path = &quot;../../tests/unit/stylo&quot;}
20:58Manishearthfroydnj: yeah features will propagate
20:58Manishearthcargo test -p stylo_tests --features blah blahblah
20:58froydnjoh, huh. interesting
20:59Manishearthmight want to test this
20:59froydnjbut those --features would have to be listed in stylo_tests&#39;s Cargo.toml, no?
20:59froydnjI was going to work on that tomorrow, today has been Cargo hacking and watching flat tires get fixed
20:59cbrewsterbz_away: hmm it might be worth doing something like that in Servo
21:00Manishearthfroydnj: I don&#39;t think so. I *think* `cargo test -p stylo_test --features foo` uses the features on the main crate
21:00froydnjManishearth: whoa, nifty
21:01Manishearthfroydnj: tbh I&#39;m not sure how cargo handles this kind of &quot;extra&quot; test crate
21:02Manishearthfroydnj: here, I&#39;ll experiment with it :)
21:04bzcbrewster: It&#39;s a bit more of a pain in servo because there&#39;s no centralized &quot;list of all the properties&quot;
21:04Manishearthfroydnj: yep. cargo test -p foo uses the features from the toplevel crate
21:05froydnjis it just me, or is the BTreeMap Entry api a little impoverished?
21:05bzcbrewster: It&#39;s scattered over a bunch of mako files
21:05froydnjManishearth: excellent
21:05* froydnj cannot find an efficient way to do what he wants
21:05bzcbrewster: and I&#39;m not sure how well those mako files work for generating something other than Rust code....
21:05* cbrewster has no clue either
21:05gwajeffrey: sorry for missing that review - r+&#39;ed
21:05bzcbrewster: But yes, it is generally better to not have multiple lists that have to be kept in sync in non-obvious ways
21:06cbrewsterAgreed
21:10linclarkquick debugging question, let me know if I should ask in a different channel...
21:10linclarkI&#39;m trying to step through Stylo in Firefox, but it only hits my breakpoints when I&#39;m loading a page like about:config. I assume that&#39;s because that page runs in the chrome process. How can I step through the Stylo with a page running in the content process?
21:13gwlinclark: I have minimal experience with debugging gecko, but when I did I disabled e10s altogether, so that it was all running in a single process. There&#39;s probably a better method that people who work on stylo will know though!
21:13bzlinclark: You have to attach your debugger to the content process
21:13bzlinclark: or open a new non-e10s window and load your page there
21:13bzOr yes, disable e10s altogether...
21:13bzUsually &quot;open in non-e10s window&quot; from the tab context menu is pretty simple
21:14linclarkoh yeah, I guess disabling e10s will probably be the easiest route... thanks for the suggestion :)
21:14bzIt can be nice to do it on a per-tab basis so you can debug just the thing you care about, sometimes
21:15* gw takes notes
21:20linclarkthat worked, thanks! I didn&#39;t even realize that there was a &quot;open in non-e10s window&quot; option in the tab context menu
21:25emiliobholley: landed a bunch of patches that speed up stylist rebuild times. I tried to reuse allocation space using HashMap::clear() instead of foo = HashMap::new(), but that didn&#39;t seem to improve much.
21:25emiliobholley: at this point I think the gross of the work to speed that up is either make atom refcounting faster, or share cascade data across rebuilds (which heycam self-assigned last night)
21:26emiliobholley: so let me know if I should keep look into the atom thing, help Cam (if he needs it) or I should go fixing and diagnosing other bugs
21:27emiliobz: Opinions on ^ also welcome
21:27emilio(atom refcounting is bug 1362338, fwiw)
21:27firebothttps://bugzil.la/1362338 NEW, nobody@mozilla.org Consider devirtualizing refcounting of nsIAtom
21:28fwiw:)
21:28emiliofwiw: whoops :-)
21:28fwiwwoops woops ;)
21:43avadacatavraomg my build is taking foreveeer today
21:46BoddlnaggHey there! Just wanted to tell you that the logs are not working (logs.glob.uno is down?)
21:54emilioBoddlnagg: huh, good catch... glob seems away though, which is who hosts that webpage afaict
22:05bholleyemilio: how do our numbers look after your patches?
22:07emiliobholley: They shave about 300ms to half a second in the microbenchmark (though there&#39;s a bit of noise). I had a Facebook profile, but I&#39;m back home so not on my normal computer, will share tomorrow (or, you can reprofile too with all those applied I guess)
22:08bholleyemilio: 300-500 out of how much total?
22:09emiliobholley: the amount of time spent in rebuilding itself was about 2/3s IIRC, though I don&#39;t have the numbers right here. Basically the patch that really should improve both that micro-benchmark and general incremental rebuild times is the bug heycam started working on (which I planned to take, but he was just faster)
22:10emiliobholley: so I instead removing a bunch of dumb stuff we were doing in hot paths
22:10emilio*removed
22:11bholleyemilio: that is great news!
22:11bholleyemilio: have you determined whether we&#39;re flushing more often than gecko?
22:11bholleyemilio: I really seemed to be seeing that
22:11bholleyemilio: but don&#39;t recall a testcase offhand
22:15emiliobholley: I think we&#39;re not... But I could log it and find out tomorrow I guess The dumb stuff I expected us to do is something like flushing the user font set, then modifying stuff, then styling, instead of the other way around, but I don&#39;t think that&#39;s the case.
22:15emiliobholley: basically, all the cases I&#39;m aware we were rebuilding more stuff than needed were fixed in bug 1386602
22:15firebothttps://bugzil.la/1386602 FIXED, emilio+bugs@crisal.io stylo: We rebuild stylesheets most of the time when we don&#39;t need to when media query affecting valu
22:16emiliobholley: but please ni? me and I&#39;ll log a bit tomorrow so I can verify it
22:16* emilio is going to sleep for the dat
22:16emilio*day
22:18emiliobholley: I think it&#39;s still worth it to speed up atom refcounting fwiw
22:18emiliobholley: it appears in every profile with somewhat high numbers
22:18emiliobholley: and should be an easy win
22:22xidornbholley: how much do we care about tests on mac?
22:30bholleyxidorn: we probably care about windows more
22:30xidornbholley: context is that I was going to enable more mochitests in bug 1383992, while jmaher is concerned about this would increase the load on osx platform
22:30firebothttps://bugzil.la/1383992 NEW, xidorn+moz@upsuper.org stylo: Retire stylo-failures.md and enable more mochitests which don&#39;t fail a lot
22:30bholleyxidorn: can we make them central-only?
22:30xidornbholley: and mark them T3?
22:31xidornwe can, but wouldn&#39;t that be annoying that some issues only appear on central?
22:31bholleyxidorn: well, maybe just explain the situation to the sheriffs and say this is only for a few weeks, and if they merge bustage across on those tests they can disable and NI
22:32bholleyxidorn: we already have this issue in some ways because we aren&#39;t running some tests on inbound
22:32xidornbholley: hmmmm
22:32xidornbholley: ok, I&#39;ll check with jmaher and see if he&#39;s happy with that
22:32bholleyxidorn: (check with the sheriffs too :-) )
22:37xidornbholley: btw that also means we would regress the test coverage on mac platform because we are retiring the mochitest-style and mochitest-chrome-style
22:37xidornwhich currently run on autoland
22:37bholleyxidorn: I think if we&#39;re running on windows and linux, and mac on m-c, that&#39;s probably ok
22:37xidorn(I don&#39;t think we are running on windows, though)
22:37xidornanyway... ok
22:38bholleyxidorn: oh. Will windows have the same problem? Or is that AWS?
22:38bholleyxidorn: i.e. can we just spend more money on windows
22:41jryansxidorn: bholley: for windows, we are trying to pre-seed the SETA system that runs jobs only some of the time to reduce load, see bug 1386806
22:41firebothttps://bugzil.la/1386806 NEW, nobody@mozilla.org seta data for new win32/win64 stylo tests
22:43xidornjryans: I wonder whether we can do the same thing for new mochitests on mac so that we don&#39;t need to disable them...
22:43KWiersobholley: I heard my name
22:44jryansxidorn: hopefully, it would be sad to disable the already running tests...
22:44xidornjryans: btw I don&#39;t think we should include macos in our default try syntax...
22:44bholleyKWierso: basically, we&#39;re running into capacity concerns turning on mac tests, how do you feel about us cheating a bit for a few weeks?
22:44xidornKWierso: see bug 1383992 comment 9
22:44firebothttps://bugzil.la/1383992 NEW, xidorn+moz@upsuper.org stylo: Retire stylo-failures.md and enable more mochitests which don&#39;t fail a lot
22:46KWiersobholley/xidorn: if they&#39;re tier-3, no one will be watching them at all from the sheriffing side
22:46jryansxidorn: ah, with the try syntax i was just trying to be exhaustive for everything stylo related...
22:46bholleyKWierso: I&#39;m suggesting keeping them tier-1
22:46bholleyKWierso: but only running them on central, since apparently we don&#39;t have capacity to do better
22:47bholleyKWierso: we&#39;ll still have full continuous coverage on windows/linux, and on non-stylo
22:47jryansxidorn: i assume people know they don&#39;t have to run all platforms if its not needed...? maybe i&#39;ll add some more notes on the wiki
22:47KWiersobholley: if you want tier-1, they need to be running on all integration branches and m-c, per the visibility policy
22:47bholleyKWierso: so the breakage would only come if there was some stylo+mac specific issue
22:47bholleyKWierso: I&#39;m asking to relax the policy for a few weeks while we get over this hump
22:47bholleyKWierso: AFAICT we&#39;re already relaxing it by not running certain jobs on inbound
22:48KWiersotier-2 could be m-c and try exclusive, which would let you see which merge broke it, and then you could bisect on try
22:48bholleyKWierso: will the sherriffs tell us if it breaks?
22:48bholleyKWierso: if so that&#39;s basically what we want
22:48bholleyKWierso: we don&#39;t expect it to break
22:48KWiersobholley: can you send an email to sheriffs@m.o explaining it? I probably shouldn&#39;t be arbitrarily deciding things.
22:49bholleyKWierso: (sure, though it sounds like maybe tier-2 is exactly what we want?)
22:49KWiersotier-2 has sheriffs see it and file bugs for failures, but no automatic backouts, even if we see what caused it
22:49xidornthat sounds fine
22:49bholleyKWierso: yes, tier-2 sounds fine for this
22:50bholleyKWierso: thanks
22:50KWiersonp
22:50KWiersoyou&#39;ll definitely want to be able to include them on try, I think, otherwise all bisecting will have to be done locally
23:18mbrubeckHmm, Servo currently traverses the flow tree twice when building display lists... looks like it should be easy to combine into one traversal.
23:18mbrubeckIn fact our documentation lies and says it *is* one traversal.
23:25njnugh, diagnosing build.rs failures is no fun
23:26xidornnjn: what happens?
23:27njnxidorn: pages and pages of output, including numerous stack traces, as well as lots of other stuff
23:27njnxidorn: and usually no useful information about what actually went wrong, AFAICT
23:27xidornnjn: oh, bindgen failed?
23:27njnprobably?
23:28xidornbecause otherwise there should be only one stack trace
23:28njnyeah, panic under build_script_build::build_gecko::bindings::write_binding_file
23:28xidornwe run bindgen in parallel, so there could be multiple...
23:28xidornthat function isn&#39;t supposed to fail in general :/
23:29xidornnjn: what&#39;s panicking?
23:29njnxidorn: I clobbered and now it seems to be working
23:54xidornwow dmd live mode is sooooo slow...
23:54njnxidorn: be brave!
23:58xidornnjn: where would the output be saved?
8 Aug 2017
No messages
   
Last message: 14 days and 5 hours ago