mozilla :: #servo

20 Apr 2017
00:02jryansjack: added a comment to the issue, https://github.com/jryans/servo/commits/range-client-rects
00:06jackjryans: i started working on it ysterday, so i figured i'd finish up your work if you weren't going to do i
00:06jryansjack: sure, feel free to do so!
00:06jryansjack: i got a bit lost once i saw every engine return different results :S
00:59heycamManishearth: thanks for the detailed comment in the MathML font size stuff, it's helpful :)
01:01Manishearthyw :)
01:01ManishearthI'll probably distill this into a post along with the other font-size stuff so that it's documented somewhere
01:10jackso i'm running python serve in tests/wpt/web-platform-tests and testharnessreport.js is not getting substituted correctly
01:10jackie %(output)d is appearing in the output html :(
01:15tillEdge's DOM tree modernization work looks interesting https://blogs.windows.com/msedgedev/2017/04/19/modernizing-dom-tree-microsoft-edge/
01:27jackManishearth: any idea why the test hardness would be actign weird?
02:11SimonSapinbholley: do you have a git or hg branch for bug 1357973 that you could push to I can look at it with Rust syntax highlighting?
02:11firebothttps://bugzil.la/1357973 NEW, bobbyholley@gmail.com stylo: store simple selectors and combinators inline
02:16Manishearthjack 301 jgraham
02:16Manishearthit should work
02:21SimonSapinbholley: never mind, the "splinter" view is relatively readable, compared to the small box of raw diff in the "details" view
02:31heycambholley: that 2x selector matching perf improvement is amazing
02:33Manishearthheycam|away: where?
02:35tillManishearth: bug 1357973
02:35firebothttps://bugzil.la/1357973 NEW, bobbyholley@gmail.com stylo: store simple selectors and combinators inline
02:35Manishearthoh nice
02:35Manishearthso the cache fix worked. wooo
02:36Manishearthwait, 2x /gecko/ ?
02:36Manishearthniiice
02:36Manishearthoh that's the stress test
02:47xidornthe csswg is discussing about making border-* properties lists :)
03:04CJKuManishearth: ping
03:23Manishearthcjku pong ish
03:23Manishearthat gym may reply later
03:24CJKuManishearth: sure, just want to make sure, when you said "We parse the context values for fill and stroke, but not for fill-opacity", the "We" means stylo, not gecko, right?
03:26jackjgraham: so python serve doesn't do the substitution for testharnessprogress.js. running test-wpt does work. why the difference and how do i fix it? it's breaking manual testing
03:40Manishearthcjku yes
03:40Manishearthsee SVGPaint
03:41Manishearthgecko does too, of course
03:41Manishearthbut we only do it fir fill and stroke, not for the opacity ones, since theyre trickier to compute
03:59CJKuManishearth: thanks. https://bugzilla.mozilla.org/show_bug.cgi?id=1338764#c4 let's see how jwatt think first
03:59firebotBug 1338764 NEW, cku@mozilla.com stylo: support context values for SVG stroke-* and fill-*
04:19CJKuManishearth: Sorry, forget it, my comment is not correct
05:07ManishearthCJKu: I'm here now
05:08Manishearthheycam: quick r on the test expectations update?
05:14Manishearthoh nvm you already did
05:14heycamManishearth: did part 1 change?
05:15heycamor did I just forget to r+ it in the first place
05:15Manishearthheycam: nah, I made a mistake :
05:15Manishearth:)
05:15* heycam does it again for good measure
05:15ManishearthI saw an in-progress push
05:16Manishearthso I thought I'd forgotten to click yes (and that the third patch hadn't been uploaded)
05:16Manishearthbut it had in a previous push
05:23heycamManishearth: still haven't had a chance to think about OMT font metrics access. maybe tomorrow or early next week...
05:24Manishearththanks
06:31xidornheycam: could you help reviewing dholbert's patch in bug 1241623 and bug 1357117?
06:31firebothttps://bugzil.la/1241623 ASSIGNED, dholbert Use a smarter (& more speccable) emulation behavior for -webkit-gradient(linear, ...)
06:31firebothttps://bugzil.la/1357117 ASSIGNED, dholbert Serialize prefixed gradients using the same prefix they were specified with (-webkit/-moz)
06:43heycamxidorn: ok, can you redirect the review request to me?
06:56xidornheycam: I cannot redirect on bug 1241623 because dholbert seems to have draft on that
06:56firebothttps://bugzil.la/1241623 ASSIGNED, dholbert@mozilla.com Use a smarter (& more speccable) emulation behavior for -webkit-gradient(linear, ...)
06:56xidornprobably just redirect the flag on bugzilla
06:57heycamok
06:57heycamthanks
07:51jgrahamjack: testharnessresport.js? I'm not sure there's a trivial solution at the moment :/ You could change tests/wpt/web-platform-tests/resources/testharnessreport.js with s/output:%(output)d/output:"%(output)d" == "1" ? true : false/ or something
07:56jackWhen run from test-wpt it subs 0 in
08:07KiChjangdid we land the wptup?
08:07KiChjangnvm, we didn't
09:37noxkaksmet: Intermittent.
09:37noxErr,
09:37noxKiChjang left...
09:52noxSimonSapin: I'm looking at https://github.com/servo/servo/issues/15211, do we have any property value in servo that should resolve to a CSS value of a parent element?
09:52crowbotIssue #15211: {fill,stroke}-opacity should support SVG-in-OT values context-{fill,stroke}-opacity - https://github.com/servo/servo/issues/15211
10:38luisbgI am trying to do a new clean build of Servo (fresh git clone) and I am getting a build error
10:38luisbghttps://paste.fedoraproject.org/paste/0kI4JaN2sSX3UH5cAQl~KF5M1UNdIGYhyRLivL9gydE=
10:38luisbganybody else having this? any ideas?
10:40luisbgproblem looks to be in freetype
10:40luisbghttps://paste.fedoraproject.org/paste/tAXQZX8cUbH09jDCzQTlaV5M1UNdIGYhyRLivL9gydE=
10:41luisbgthis is the version of freetype2 I have
10:41luisbg$ pkg-config --modversion freetype2
10:41luisbg18.3.12
10:42travis-ciServo failed to build with Rust nightly: https://travis-ci.org/servo/servo-with-rust-nightly/builds/223892522 CC nox, SimonSapin
10:47Aryxnox: hi, mochitest failure for 16511: https://treeherder.mozilla.org/logviewer.html#?job_id=92979799&repo=autoland
10:48noxUgh.
10:49* nox blames the fact that this isn't a wpt test.
10:49noxAryx: Will look into them.
10:50Aryxthank you
10:50noxAryx: "expected 105 failures but got 0" is good though, right?
10:51Aryxyes yes
10:52noxGood.
10:54noxAryx: Can you change the expectations while I look at it?
10:55Aryxi can
11:04noxAryx: What's the problem with -webkit-radial-gradient(circle 10px, red, blue)?
11:04noxI'm not sure how it's illegal.
11:12noxOh I see, in the legacy syntax it's either numbers and percentages without a shape keyword, or a shape keyword with a size keyword;
11:12noxbut not a shape keyword and explicit numbers and percentages.
11:14Aryxseems to be [pos, [shape,]] color, color
11:14noxAryx: What?
11:15Aryxnothing, ignore me
11:15noxThe problem is that circle 10px is ok in modern, but bad in legacy.
11:15noxThat being said, I just realised I don't think we parse 10% ellipse 10% in modern syntax either.
11:17noxI feel like I will have to revert one of my commits that share more code between legacy and modern syntaxes. :(
12:07ferjmhow are these lists generated https://dxr.mozilla.org/mozilla-central/source/layout/reftests/scoped-style/reftest-stylo.list ?
12:07ferjmor in other words, how can I update tests expectations for stylo?
12:13jryansferjm: it's okay to edit the stylo ones to correct test changes. however https://bugzilla.mozilla.org/show_bug.cgi?id=1351548 is about to merge them back into the main list
12:13firebotBug 1351548 ASSIGNED, slyu@mozilla.com Merge reftest-stylo.list back to reftest.list
12:13noxjryans: Do you mean it's written by hand..?
12:14jryansnox: generated by script, updated by hand as we fix things, i believe
12:14ferjmjryans: ok, thanks!
12:30jackjgraham: can you explain why the solution is nontrivial?
12:30jackwouldn't it be enough to inject a simple substituter that just knows about the output variable?
12:34jgrahamjack: Well I mean that is requires code changes
12:34jgrahamjack: But I think the solution I proposed will solve it, no?
12:34jackjgraham: i guess i am willing to make some if it means our testing tools actually work
12:35jgrahamYeah, agreed that this should be fixed :)
12:35jackjgraham: i can make that change, sure, but i don't know what the correct values are
12:35jackwhen actually used in test-wpt the value of output is 0
12:36jacki can't even find what output does. there seems to be no code docs anywhere :(
12:36jgrahamjack: Yeah it's a boolean formatted as an int so it will either be 0 or 1
12:38jgrahamIt does seem to have avoided being documented :/
12:38jgrahamIf it's truthy then we output the HTML table. Otherwise we don't in order to save time
12:38jackjgraham: i don't mena just output, but i didn't see any comments in serve or wptserve when i was poking around in there trying to find out if it was running a template engine or not
12:39jackit's possible i'm not looking in the right places though
12:39jgrahamjack: This specific thing is just python string formatting
12:39jgrahamIt's a wptrunner feature
12:39jgrahams/feature/beahviour/
12:40jackwhere does it get the config from? i thought maybe config.json but that didn't seem to be true
12:40jackin general, is there an overview of how all this fits together somewhere?
12:40jgraham(the server does have a different template system for test files: http://wptserve.readthedocs.io/en/latest/pipes.html#sub
12:42jgrahamjack: This is handled in wptrunner: https://github.com/w3c/wptrunner/blob/66a5cc86a9f8e21e8d0e7c5b9657120ccf19461b/wptrunner/environment.py#L184
12:43jgrahamIt isn't particularly well documented, but this is a special case.
12:43jackok. so if i run manually, i don't see the results table. i assume that's because output: false?
12:43jgrahamYeah
12:43jackthat is a little unfortunately for JS tests that don't output anything to the document :)
12:44jgrahamjack: Actually it looks like we changed this at some point so you don't need a special testharnessreport.js actually checked in
12:44jgrahamBecause wptrunner is providing its own
12:44jgrahamSo you can probably just overwrite the entire file with the one from upstream
12:44SimonSapinnox: not sure i understand your question, but any property can take the 'inherit' value
12:45noxSimonSapin: Am looking at https://github.com/servo/servo/issues/15211,
12:45crowbotIssue #15211: {fill,stroke}-opacity should support SVG-in-OT values context-{fill,stroke}-opacity - https://github.com/servo/servo/issues/15211
12:45jgrahamjack: https://github.com/w3c/web-platform-tests/blob/master/resources/testharnessreport.js
12:45noxtrying to understand what these additional values are supposed to compute to.
12:45SimonSapinnox: looks like that is opentype only?
12:46jackjgraham: that made it better :)
12:47jackjgraham: now i am to the problem where Range.getClientRects has totally different behavior in every browser :)
12:47jryansjack: it's fun :)
12:48jackit looks like some of it is rounding issues
12:48jackssert_equals: X coordinate expected 269.3666687011719 but got 279.3666687011719
12:48jackfrom firefox
12:48jackassert_equals: Width expected 162.625 but got 142.625 from chrome
12:48jgrahamjack: At least that one isn't my fault :)
12:48jryansjack: i think i also saw margin / padding being treated differently, either included or excluded depending on engine
12:48jgraham(as far as I know)
12:48jackjgraham: no, this one is where i was trying to be :)
12:49jackjryans: my servo impl currently returns an empty vec for this case, which is even wronger :)
12:49jryanshaha
12:57jackassert_equals: Width expected 164 but got 144 from safari
12:57jackso looks like safari and blink agree.
12:58jackand firefox has different behavior but also messes up the rounding or something
12:58jryansjack: i can't remember what edge does, but i remember starting a VM to check ;)
12:58jacki'll check when i'm back home this weekend.
12:59jackoh wait. firefox isn't rounding wrong. it's doing the opposite.
12:59jackit gets 10px more instead of 20px less
12:59jackthat's amusing
13:10jackjryans: so getboundingclientrect says it retrives border boxes, but safari and chrome are definitely returning cnotent boxes
13:11jryansjack: yep, sounds about how i rememeber it
13:11jryansjack: i wonder if libraries like codemirror that use this special case each UA...
13:12jryansor maybe there is just no padding in their usage...
13:12jackyou you call element.getBoundingClientRect() in chrome you get border box. when you call range.getBoundingClientRect() you get content box.
13:12jackthat seems like a bug
13:13jryansyeah, seems wrong to me too
13:13jack"For each element selected by the range, whose parent is not selected by the range, include the border areas returned by invoking getClientRects() on the element."
13:13jackso they are supposed to match.
13:14jackso at the very least, chrome and safari do not do what the spec says.
13:14jackit appears firefox just does something random.
13:14jackbecause insteado f the border box, it is returning border box + 10px, which is 8px mroe than the margin box :)
13:14jacker, 6px more
13:16jryans:)
13:17jacknm. that hte x coord. firefox also seems to return content box thought
13:17jackthe test is just failing on an earlier assert
13:17jryansnot seeing an existing chrome bug about it, but there are lots of other edge cases filed https://bugs.chromium.org/p/chromium/issues/list?can=2&q=getBoundingClientRect&x=m&y=releaseblock&cells=ids
13:18jackso basically all the browsers implemented the spec incorrectly, despite assumedly having getBoundingClientRect implementations on which to base this implementation.
13:20jryansi feel like this API must not get much usage, or you'd hear more noise about the differences... (though maybe the differences are why it's not used...)
13:27avadacatavrahttps://irccloud.mozilla.com/pastebin/3cG05Bze/
13:27avadacatavra^i don't think the destroy is working properly
14:11emilionox: are you looking at the radial-gradient failures?
14:36waffleshey emilio, so, we should avoid memmove'ing large types (if possible), right?
14:36wafflesdoes that mean we should clone them away?
14:38emiliowaffles: cloning is memmoving for copy types
14:38wafflesemilio, yes, but if we don't impl Copy
14:38waffles?
14:38emiliowaffles: too
14:38emiliowaffles: it's just more explicit
14:38emiliowaffles: (which is good, except if it becomes annoying)
14:39wafflesemilio, oh, I get it
14:39wafflesthanks
14:39* waffles doesn't know much C stuff
14:41avadacatavra ld: symbol(s) not found for architecture x86_64
14:41avadacatavrahas anyone gotten an error like that lately?
14:42katsavadacatavra: what are you trying to do?
14:42avadacatavrakats: compile :(
14:42* avadacatavra gets more error output
14:43avadacatavraerror: linking with `cc` failed: exit code: 1
14:43avadacatavrahttps://irccloud.mozilla.com/pastebin/sPGAFZo2/
14:43katsavadacatavra: what are you compiling? servo?
14:44avadacatavrakats: yup
14:44avadacatavraeverything was fine yesterday
14:44katsah, can't help you then
14:44avadacatavrano clue what i did
14:53avadacatavrahmm i think it might be because i was using .cargo/config to replace a crate?
15:09avadacatavrathat seems to have been the issue
15:23larsbergPSA: preparing to do a highstate to roll out https://github.com/servo/saltfs/pull/638 to the cross builders
15:24crowbotPR #638: Set JAVA_HOME env var to OpenJDK 8 for Android builds - https://github.com/servo/saltfs/pull/638
15:43* avadacatavra took a day to realize that her problem was something entirely different
15:44larsbergdeployed, buildbot & cross builders seem online :-)
15:44larsbergNow just need a PR to test the android stuff...
15:44* larsberg goes to @bors-servo try something
16:02ferjmare user and UA sheets being parsed by stylo?
16:04ferjmemilio: ^ maybe you know
16:04emilioferjm: yes
16:04emilioferjm: thought with the wrong level IIRC, let me find a link
16:04emilioferjm: bug 1321754
16:04firebothttps://bugzil.la/1321754 NEW, tlin@mozilla.com stylo: UA style sheets parsed with author features should be added at the UA level
16:05emilioferjm: but most of them should be right I think
16:05ferjmemilio: so I guess I should see some stylesheets here https://dxr.mozilla.org/servo/source/components/style/stylist.rs#227
16:05noxemilio: Yes I am.
16:05noxemilio: I fixed one of them, working on the other kind of failures.
16:06emilioferjm: nope
16:06emilioferjm: they're added as part of the normal `doc_stylesheets`
16:07emilioferjm: note that `Stylesheet` has an `origin` field
16:09ferjmemilio: ah, ok, I see. I am trying to figure out why my patch for @-moz-document only works for author stylesheets. Do you know where would be the right place to evaluate this rule for user and UA stylesheets in stylo?
16:10emilioferjm: I bet the loader has no document pointer for UA sheets (or, there's no loader)
16:11emilioferjm: IIRC all stylesheets should go through http://searchfox.org/mozilla-central/source/servo/ports/geckolib/glue.rs#504, or Servo_Stylesheet_ClearAndUpdate
16:12emilioferjm: but the second is only for @import
16:14noxjack: https://github.com/serde-rs/serde/releases/tag/v1.0.0
16:14noxajeffrey: "Zero-copy deserialization"
16:18emilioferjm: alternatively, you could _not_ evaluate -moz-document rules at parsing, but in Stylist::update
16:18emilioferjm: (given @-moz-document rules should always be at the beginning of the stylesheet, it should be cheap enough)
16:27ajeffreynox: yup, cool.
16:27ajeffreynox: orthogonal to reducing copy on serialization to ipc channels,
16:28ferjmemilio: ok, thanks for the pointers!
16:28ajeffreyI wonder if we can make use of it in servo?
16:28noxNot really orthogonal no, this means bincode can borrow stuff into the deserialised value from its given buffer.
16:29ajeffreynox: indeed, doesn't reduce copy on send though :/
16:29ajeffreyreduces it on recv.
16:46eddybGankro: there you go :P https://github.com/rust-lang/rust/pull/41421
16:46crowbotPR #41421: rustc_trans: do not treat byval as using up registers. - https://github.com/rust-lang/rust/pull/41421
16:47Gankroeddyb: thanks!
16:51froydnjdo infra builds for servo go through the relevant mach commands, or are the infra bits defined someplace else?
16:52froydnjoh, excellent, it looks like they do
16:55Manishearthfroydnj: everything except a couple of scripts in etc/ci
16:56Manishearthlockfile_changed, manifest_changed, and check_intermittents
16:56Manishearththe first two test if certain files changed during the build and output a diff
16:57Manishearththe last one is a wrapper around mach that tests if known intermittents are still intermittents
16:57Manishearththey're all kinda meta-tests so they don't have a mach command (add one if you want)
16:57Manishearthfroydnj: etc/ci/buildbot_steps.yml is the full set of CI steps
17:08* froydnj discovers that he has to split this PR into two parts
17:12Gankrohey pcwalton got some time to discuss $ANCIENT_ARCHITECTURAL_DECISIONS in webrender? No one seems to recall why aux lists exist anymore.
17:13GankroLike obviously we get what they do, it's just not clear why they're split off from display lists
17:13pcwaltonGankro: aux lists exist in order to allow display lists to be transferred via shmem
17:13pcwaltonand if not transferred via shmem, in order to make serialization/deserialization faster
17:14pcwaltonbasically everything WR sends needs to be POD
17:14Gankropcwalton: why is it faster?
17:14pcwaltonbecause otherwise serde is invoked which is slow
17:14pcwaltonby slow I mean
17:14pcwaltonquite a bit slower than the entire rendering process
17:14pcwaltonbefore aux lists
17:14pcwaltonusually DL sending was slower than (layout + styling + painting) all combined
17:14Gankropcwalton: aux lists are currently the same size as display lists in servo
17:14pcwaltonso?
17:14pcwaltonIm not talking about size
17:15pcwaltonIm talking about serialization overhead
17:15pcwaltonthe time it takes to serialize
17:15Gankropcwalton: as in, display items are much more complex, and so serde chokes on them?
17:15pcwaltonyes
17:17Gankropcwalton: did you ever look at the details of why serde is slow or try to optimize that? (is that a dead end for me to explore)
17:17pcwaltonwell, you will introduce a regression no matter what
17:18pcwaltonbecause right now the serialization overhead is 0
17:18pcwaltonany serialization overhead is more overhead than zero :)
17:18Gankropcwalton: it's also unsound; you can't trust another process to send you bytes you can transmute
17:18pcwaltonsure you can, if its POD
17:18pcwalton(modulo floating point whatevers)
17:19Gankropcwalton: there's enums
17:19Gankroand bools
17:19Gankroif transmute::<bool>(3) { } can do whacky wild things
17:19pcwaltonthat hasnt caused problems in practice
17:19Gankropcwalton: we aren&#39;t being attacked in practice?
17:20GankroThe whole point of multi-process is to isolate a compromise
17:20noxIf it&#39;s unsound, it needs to go.
17:20pcwaltonyeah, so I agree it is scary
17:21pcwaltonif you can rework it so that the regression is minimal, Im fine with it
17:21Gankropcwalton: so what I&#39;m looking at right now is moving displaylists to bincode; it does indeed regress perf a lot, but I&#39;m currently looking at getting wins elsewhere for doing it
17:22GankroIn particular DisplayItems are *huge* because of a couple variants, so bincode actually saves a ton of memory
17:22pcwaltonyeah, Im very uncomfortable with that perf regression
17:22pcwaltonhave you tried something like abomonation?
17:22pcwaltonI tried bincode before
17:22GankroSo if we never actually construct a Vec<DisplayItem> we can potentially save a ton of space, and therefore time
17:23pcwaltonIm skeptical that will win enough
17:23Gankropcwalton: it takes half the time to physically send a DisplayList when it&#39;s bincodeified
17:24pcwaltonbut how long is that compared to the overhead of serialization?
17:24pcwaltonhow about this
17:24pcwaltonis there any way you can bincodeify on the fly?
17:24Gankropcwalton: yes
17:24GankroThat&#39;s the plan
17:24pcwaltonas in, the temporary DL should never exist in the first place
17:24Gankroyes
17:25pcwaltonok, that may not regress perf very much if at all
17:25pcwaltonif you do it right
17:25pcwaltonIm ok with that solution
17:25pcwaltonjust measure perf :)
17:25Gankropcwalton: yep, making sure to -- although this plan is harder since it shifts costs around
17:25pcwaltonalso, you will want to deserialize on the fly too
17:25pcwaltonIm not sure how best to do that
17:26Gankropcwalton: bincode supports it
17:26pcwaltonok, cool
17:26GankroBuildDisplayList will just serialize_into, and DisplayListTraversal will deserialize_into
17:27pcwaltonok, cool
17:27pcwaltonthats what Chrome does for example
17:27Gankropcwalton: if I kill aux_lists, then firefox can also do its display_list concatenation as actual byte concatenation
17:27pcwaltonso if you can do it that way without much of a perf regression its fine by me
17:27GankroWhich is why I got to &quot;are aux lists actually useful&quot;
17:27pcwaltonah, I see
17:32Gankropcwalton: in theory I can also recycle the aux list vecs since we only ever deserialize one DisplayItem at a time from what I&#39;ve seen
17:32GankroAlthough this is Harder
17:33pcwaltonnice, yah
17:33pcwaltonyeah
17:33pcwaltonbasically making things on the fly as much as possible is important
17:33pcwaltonDL serialization overhead is something Chrome struggled with for a long time, just fwiw
17:34Gankropcwalton: do you know what they did?
17:34pcwaltonjust extreme microoptimization I think
17:34pcwaltonin theory we should be in better shape because CSS primitives tend to be smaller than vector commands
17:34Gankropcwalton: also just to double check -- even with shmem both ends need to construct a Vec<u8> and copy that into the shared buffer, right?
17:35pcwaltonyeah, unless you can statically calculate the size in advance
17:35Gankropcwalton: that... might be possible actually?
17:36Gankropcwalton: we probably want an upper bound to prevent DDOS from a malicious process
17:36GankroBut that might be too big?
17:36pcwaltonyou might have to modify ipc-channel a bit to allow for this (some sort of Builder interface)
17:36pcwaltonipc shmem is also set up that way as a simple way to statically prevent races
17:36pcwaltona Builder interface could fix that
17:36Gankropcwalton: the receiver probably needs to copy anyway, to guard against a malicious races, yeah
17:38Gankropcwalton: are there any practical limitations to how big you can make a shmem buffer?
17:39pcwaltonitd be kernel limitations
17:39pcwaltonI dont think Linux or Mac have any notable limitations there
17:41Gankropcwalton: would it also be possible to cut out the message send the aux_list is part of? Or is that necessary for other things.
17:42pcwaltonnot sure what that means
17:42pcwaltoncan you link to the code in question?
17:45Gankropcwalton: oh sorry I was misremembering
17:45Gankrothinking of https://github.com/servo/webrender/blob/master/webrender_traits/src/api.rs#L318-L325
17:45Gankropcwalton: it&#39;s a bit weird that we send two messages
17:46pcwaltonGankro: oh yeah, thats just to avoid invoking serde on the first message
17:46pcwaltonerr
17:46pcwaltonto avoid invoking serde on the second message
17:50Gankropcwalton: oh! I might be able to make all of the DisplayItems store &[T] instead of Vec thanks to the new zero-copy stuff in serde!
17:50pcwaltonoh nice, is that specialiation based?
17:50GankroOh wait that&#39;s only strings bah
17:50pcwaltonspecialization
17:51Gankropcwalton: no clue on the details -- TyOverby is the one to ask
18:00TyOverbyGankro: zero-copy only works for &str and &[u8]
18:00GankroTyOverby: yep, totally makes sense
18:00GankroTyOverby: although now it&#39;s making me theory craft about two phase deserialization....
18:00TyOverby(no reason to assume that it couldn&#39;t support &[u32] and other primitives eventually though)
18:01TyOverbyGankro: are you just trying to get rid of that Vec allocation?
18:01GankroTyOverby: yeah; there&#39;s two ways I could do it
18:01TyOverbyI&#39;m thinking of one way, what&#39;re your plans?
18:02GankroTyOverby: the nicest would be being able to pass the deserializer a few &quot;you might need these&quot; Vecs that I can recycle on each iter
18:02TyOverbyah, so it would reuse the memory?
18:02GankroTyOverby: yeah
18:02GankroI only deserialize one item at a time
18:03TyOverby^ is that your 2nd option?
18:03GankroTyOverby: I haven&#39;t thought about this long enough to order the ideas
18:03GankroTyOverby: the other would be having the types &quot;have&quot; a &[u8] that is actually more serialized data
18:04noxGankro: We discussed in the past having a Deserialize::deserialize_into trait method,
18:04noxthat would be similar to Clone::clone_from,
18:04noxI&#39;ve yet to implement it though.
18:05nox(That was regarding the &quot;you might need these&quot; Vecs.)
18:05TyOverbyGankro: what I would do is: instead of serializing a Vec<T>, I&#39;d serialize all my T individually into the same writer. Then on the other end, I could deserialize each element out individually as well
18:05GankroTyOverby: that sounds like aux lists
18:06TyOverbywhat&#39;s an &quot;aux list&quot;?
18:06GankroTyOverby: all the Vecs that would be in DisplayItems are currently seperately cordoned off into mega &quot;aux lists&quot;
18:06GankroSo all the Vecs are one big Vec, and each DisplayItem has slice indices for where its Vec is
18:07TyOverbynah, this is way simpler. This is just taking advantage of the fact that Bincode doesn&#39;t mess with the remainder of a buffer after it reads an element out of the buffer
18:07TyOverbyGankro: you should be able to do both though
18:08GankroTyOverby: would your solution allow me to concatenate two DisplayLists constructed in this manner without fixups?
18:09TyOverbywhat&#39;s the type of DisplayList?
18:11GankroTyOverby: Vec<DisplayItem>
18:11TyOverbyGankro: Oh then yeah, that&#39;s easy
18:12TyOverbydoes the segmentation matter?
18:12Gankro(to be clear, I only want to ever have a Vec<u8>, and simulate a Vec<DisplayList> by serializing/deserializing for push/iter)
18:12GankroTyOverby: I don&#39;t know what that means
18:12TyOverbythat is, is there a difference between two Vec<DisplayItems> and one Vec<DisplayItem> that contains all the elements?
18:13GankroTyOverby: today there is because of the aux lists -- the offsets into the auxlists need to be updated when you concatenate. I am looking at killing auxlists and moving the sub-Vecs back into DisplayItems, though.
18:14GankroWhich is why I want some way to reuse allocations
18:14TyOverbyeither way this is possible, but if you kill AuxLists then it&#39;s a piece of cake
18:15GankroTyOverby: why would dumping the sub-vec data after the DisplayItem be easier than putting it in an actual Vec in the DisplayItem?
18:15GankroJust less interference with middle of deserialization?
18:16TyOverbyGankro: yeah, basically
18:17GankroTyOverby: so I&#39;d do like deserialize_display_item(); if deserialized_item is variant_x { deserialize_list_of_t(); }?
18:17TyOverbyYou can deserialize &[u8] into Iterator<T> pretty easily, but it&#39;s harder to go to Iterator<Iterator<T>>
18:17TyOverbyBecause there&#39;s no way of knowing where the gaps are, so you&#39;d need to do some work upfront to know that information
18:18GankroTyOverby: how is the start/end of a Vec indicated in bincode?
18:18TyOverbyGankro: the length is given at the start
18:19GankroTyOverby: doesn&#39;t that mean you can&#39;t byte-concat two Vecs?
18:20TyOverbyGankro: right, but what you _can_ do is serialize all the elements from both vecs individually
18:21TyOverbythen you can call deserialize_from multiple times on the same Reader and get your elements out whenever you need them
18:22GankroTyOverby: what if I serialized the two independently and want to concat them without deserializing
18:22TyOverbyit would be harder, but you could still definitely tease the two vecs apart lazilly whenever you want
18:23GankroTyOverby: if the top level object is a Vec<T>, is there any reason to store len?
18:23TyOverbynot if you aren&#39;t going to concat them
18:24GankroTyOverby: ok so I can just have a Vec<u8> that I just repeatedly serialize_into(display_item)
18:24TyOverbyyep
18:24GankroAnd then I can concat two of those without cleanup
18:24TyOverbyexactly
18:24GankroDang bincode is nice
18:25TyOverbyand then you just call deserialize_from as many times as you want from the other side
18:25GankroTyOverby: how do I know when I&#39;m &quot;done&quot;?
18:25GankroIf deserialize returns Err?
18:25TyOverbyGankro: you&#39;ll get a BincodeError::IoError(EOF)
18:26TyOverby(or, if you are decoding from a slice, you could just see if that slice has length 0 )
18:27TyOverbybecause bincode will always end on exactly an object boundary when reading from a Reader
18:31GankroTyOverby: since Serde makes that check for me, I might as well let it handle it.
18:58* avadacatavra finally has a new error!
19:08TyOverbyGankro: yeah, might as well
19:40gwjack: edunham: would it be possible to up the priority of https://github.com/servo/saltfs/issues/614 ?
19:40crowbotIssue #614: Provision a machine for running WR benchmarks. - https://github.com/servo/saltfs/issues/614
19:41edunhamgw: yep, thanks for the ping
19:41gwedunham: cool, thanks!
19:42avadacatavrahttps://irccloud.mozilla.com/pastebin/isF2gevC/
19:42avadacatavrahttps://irccloud.mozilla.com/pastebin/C5nYIHP9/
19:43avadacatavrai&#39;m getting this backtrace. it looks like there&#39;s something wrong with cloning the MutableOrigin?
20:11KWiersonox, emilio: any progress on the stylo test failures on autoland?
20:11noxI can make a PR for the first fix I wrote, but weren&#39;t the expectations changed meanwhile?
20:12KWiersonox: everything&#39;s still failing at https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=3864467a616c7ac6e4801b8860ec38177f0d09f7&noautoclassify&filter-searchStr=stylo%20(s)
20:12noxAh...
20:12noxI thought whoever reported it to me changed them.
20:12KWiersoit&#39;d settle for an expectations adjustment for now
20:13noxIt?
20:13KWiersoI&#39;d*
20:13noxGive me one hour and I can make a PR for the 2 bugs I identified.
20:14jryansnox: KWierso: should i tweak the expectations for now?
20:16bholleyjryans: yes, please
20:16jryansok!
20:19bholleyemilio: you around?
20:21jryansKWierso: okay, hopefully updated now!
20:21KWiersojryans: thanks
20:24emiliobholley: yup!
20:25bholleyemilio: oh great
20:25bholleyemilio: can you stamp the last patch in bug 1357973?
20:25bholleyemilio: trivial
20:25firebothttps://bugzil.la/1357973 NEW, bobbyholley@gmail.com stylo: store simple selectors and combinators inline
20:25emiliobholley: looking
20:25Manishearthbholley: serialization should always happen on main thread, yes?
20:26bholleyManishearth: I believe so
20:26Manishearthcool
20:27bholleyManishearth: though it would be nice not to require that if we don&#39;t already, since .e.g. it allows debug logging during the parallel traversal
20:27bholleyManishearth: but it depends what kind of serialization we&#39;re talking about
20:27bholleyManishearth: and whether we already would crash if it happened OMT
20:28Manishearthbholley: font face rule serialization
20:28Manishearthit calls down to nsCSSValue serialization
20:28Manishearthwhich won&#39;t work off main thread
20:28bholleyManishearth: I guess it&#39;s probably fine
20:28bholleyManishearth: we can always set sequential mode to get our logging
20:28Manishearththough technically in this case it should be fine since font faces don&#39;t contain colors
20:28Manishearthcool
20:32noxbholley: What do you think of https://github.com/servo/servo/issues/16543?
20:32crowbotIssue #16543: Port test_value_storage.html and test_property_syntax_errors.html to WPT tests in /_mozilla/ - https://github.com/servo/servo/issues/16543
20:32Manishearthbholley: oh lol I should have actually read the bug comments that told me to do exactly this
20:32Manishearth(I was going through the failures on the side, without looking at the bugs)
20:33bholleynox: I don&#39;t think we should do it
20:33bholleynox: you can run the tests locally by just building sytlo
20:33noxWhy?
20:33bholley*stylo
20:33noxHow?
20:33nox./mach geckolib doesn&#39;t let me run these tests, does it?
20:33Manishearthno
20:33bholleynox: I mean an m-c stylo
20:33noxI would like to avoid having to work from an m-c stylo.
20:34Manishearthnox: don&#39;t; just copy over diffs and test them there
20:34Manishearthnox: use cinnabar if you don&#39;t like hg
20:34bholleynox: you can work from within the servo/ directory if the m-c stylo build
20:34Manishearth(that too)
20:34bholleys/build/checkout/
20:35Manishearthbholley: note that that only works smoothly in a cinnabar m-c clone (or gecko-dev clone), not hg, because hg doesn&#39;t like .gitignore iirc
20:35Manishearth(so your build directories will not be ignored)
20:35noxOr we could have tests for code that is in Servo, accessible from Servo, but not currently tested.
20:35bholleynox: there is an enormous amount of test coverage in m-c
20:35bholleynox: we don&#39;t have time to port it all to servo
20:35bholleynox: but servo will still benefit from it because we test it with gecko&#39;s infra
20:36Manishearthnox: a lot of these tests have been moved to css-tests. however a lot of the code we are testing is not enabled in servo mode
20:36noxManishearth: There are no tests for gradients. At all.
20:36bholleyoh, that too ^
20:36Manishearththe code is *not* in servo, it often calls into gecko stuff, and of course gecko does all the layout
20:36noxThe code *is* in Servo.
20:36noxThe fixes I did about parsing of CSS stuff definitely is reachable from the Servo side of things.
20:37Manishearthnox: port the test to servo if you want
20:37noxbholley: I&#39;m pretty sure I didn&#39;t say all of them, but only 2.
20:37Manishearthit&#39;s just going to take longer
20:37Manishearthwe don&#39;t have that time :)
20:37ManishearthIn the past I was shifting tests to wpt or csstests, and writing new tests there
20:38bholleynox: converting those tests to wpt is nontrivial, dbaron has some reasons why
20:38bholleynox: and having two not-completely-overlapping copies isn&#39;t great either
20:38noxWell if dbaron could state these reasons in the ticket that would be nice.
20:39bholleynox: the basic issue is that wpt organizes the tests by spec, and these tests are organized by CSSOM operation
20:39noxWhy are we talking about WPT now?
20:39noxI said /_mozilla/
20:39Manishearththat&#39;s servo specific
20:39bholleynox: the ideal way to share the tests would be to avoid having two copies, and share them in wpt
20:39noxThere is nothing that forces us to reorganise anything in the test if it&#39;s just to put it in /_mozilla/
20:39bholleynox: but converting them to wpt is nontrivial
20:39bholleynox: and duplicating isn&#39;t great
20:40noxAnd having failures a posteriori in autoland isn&#39;t great either.
20:40Manishearthwhich is why geckotry exists
20:40bholleynox: that is price we have to pay with the current stylo tooling, and yes, geckotry
20:40noxManishearth: geckotry doesn&#39;t gate.
20:40Manishearthsure
20:40Manishearthnox: the actual CI is WIP
20:40noxI know.
20:40bholleynox: our workflow and tooling are suboptimal to be sure
20:41bholleynox: but we&#39;re also in a huge hurry to do something that will change the world
20:41bholleynox: and I don&#39;t want to slow down because the tooling isn&#39;t perfect
20:41ManishearthI would like to move the mochitests to wpt, but there are a host of reasons why w