mozilla :: #servo

19 May 2017
00:14Manishearthsomeone want to r? #16914 ?
00:14crowbotPR #16914: Update app_units to 0.4.1 -
00:15jryansManishearth: r+
00:16xidornemilio: still around?
00:18Manishearthjryans: there are layout changes now :)
00:19Manishearthemilio: want to r? the layout changes in the PR i linked above?
00:20jryansManishearth: they seem pretty straightforward to me, r+ if that's good enough for you
00:21Manishearthjryans: cool then :)
00:23xidornit seems emilio isn't around at this time today
00:23xidornit seems emilio isn't around at this time today
00:24xidornManishearth: could you do a quick review for
00:24stshineManishearth: I guess there will be new passes for you PR
00:25Manishearthxidorn: r+
00:25Manishearthstshine: there aren't it seems
00:25xidornManishearth: thanks
00:25stshineManishearth: ugh :/
00:26Manishearthstshine: which makes sense -- there weren't any passes in the csstests in gecko either
00:26Manishearthfor stylo
00:26Manishearthand wpt doesn't really test this stuff
00:27stshineManishearth: only crashtests?
00:28Manishearthstshine: and mochitests
00:30stshineManishearth: once I fixed a bug for the overflow in servo
00:30stshineIt may be good if servo also have these tests ...
00:32Manishearthstshine: I'd like to slowly work on migrating them to csstests. it's not possible for all of them (many test specific behavior)
00:41ManishearthStylo is down to 922 reftest failures btw!
00:41xidornwhy my "git rebase" converts my commit to a merge commit...
00:41ManishearthWe knocked out over 300 this week and are now below 1k
00:42KiChjangManishearth, !!!!!!!!!!!!!
00:44xidornwho can tell me how to do git rebase with conflict...
00:44stshinexidorn: git rebase master
00:45stshinexidorn: and, if you know emacs, magit makes your life better
00:46xidornstshine: ok that works. it seems there is some bug with "-i"
00:46xidornwhich converts my conflicting commit into a merge with master
00:47Manishearthxidorn: that's because you typed git commit after you resolved
00:47Manishearthor git commit --amend
00:47Manishearthdon't do that :)
00:47xidornManishearth: I don't. I just do git rebase --continue
00:48Manishearthrebase is weird. If you are in `e` mode, git commit--amend is what you do. If in merge-conflict-fixing mode, `git add -u && git rebase --continue` is what you do, because amend will amend the previous commit
00:48stshinexidorn: you don't need -i for resolve conflicts
00:48Manishearth-i just means it will ask you about the rebase itinerary first
00:48xidornI tried "git rebase -i master" and put "reword" on the commit, and then it converts the commit to merge and change the author to bors when I do "git rebase --continue"
00:48Manishearth"interactive mode"
00:48stshinexidorn: only do that if you want to make changes to some earlier commit
00:48xidornstshine: I do want
00:49xidornbut it seems if I do "git rebase master" first and resolve conflict, and do "git rebase -i master", then things work fine
00:49stshinexidorn: then, first git rebase -i, make the changes, and git rebase master to resolve conflicts later
00:50xidornstshine: I don't think it allows you to do git rebase without a ref commit
00:51stshinexidorn: oh right. just ref to a commit in your current branch
00:51stshinexidorn: once again, magit improves your life quantity. Please have a try
00:52xidornstshine: but I don't use emacs :)
00:54stshinexidorn: IMO it worths to use emacs solely for that git interface ;)
00:55* TYLin cannot live without magit
01:11xidorndoes servo ci takes longer nowadays?
01:11xidornprobably 1.5hrs now?
01:11xidornwhich means... we would have at most 16 prs merged per day?
01:16stshineLooks like that happens after the rayon update.
01:24heycamhow long do the builds takes vs the tests?
01:28emilioManishearth: xidorn: around, do you still need me?
01:28emiliojryans: ping?
01:28xidornemilio: just wanted a quick review for change on top of the toml patch, and Manishearth has done for me
01:29emilioxidorn: oh, cool :)
01:29stshine19 mins for build, and 40 mins for test
01:30stshineemilo: rayon update to 0.7 makes servo 2x slower :(
01:30stshineemilio: ^
01:30xidornstshine: it has been 1.75hrs since bors-servo post "testing commit" on #16915, and it hasn't been merged
01:30crowbotPR #16915: Shrink selectors::Component, implement attr selector (in)case-sensitivity -
01:31emiliostshine: it's not due to the rayon update I think
01:31emiliostshine: it's because to land it we reduced the number of processes we ran on CI
01:32xidornI guess servo ci should record build time and track its change like gecko?
01:32emiliostshine: (if you mean it made it slower in other context, like some benchmark or something, I'd be happy to look into it)
01:32* xidorn thinks gecko has that, but not sure
01:33stshineemilio: gah, why the hell do we need to reduce CI processes to make it land?
01:34heycamrunning tests seems like something that should be easy enough to parallelize over more machines
01:34emiliostshine: I didn't take that decision btw :). But I think rayon was struggling in a very frequent intermittent or something like that
01:34emilioheycam: indeed
01:35* stshine panics
01:39* xidorn wonders what is p=rollup
01:39stshinexidorn: don't know what happened, but time for mac css tests increased from 20 mins to 53 mins recently. woo
01:43xidornit's 1hr57min
01:44xidornso we can only land ~12 PRs per day now :)
01:44xidornand I have two prs today
01:44cbrewsterHopefully it will clear out this weekend, still doesn't help long term though
01:45xidornthere are already 17 prs in the queue
01:47bzlarsberg is working on standing up more mac boxes and splitting the tests
01:47bzTo take the times down
01:47jryansemilio: on phone
01:47emiliojryans: np, I'll comment on the bug
01:48xidornbz: that's great news
02:46emiliojryans: I went through all the visited patches, hopefully what I said makes some sense, I'd love your feedback on the comments
02:46emiliojryans: I need to sleep now, but let me know if you want to chat about it tomorrow or w/e :)
02:49hiroI'd like to factor out a function that returns an Iterator, how can I do it? I did specify [feature(conservative_impl_trait)] in components/style/, but I got an error; [feature(conservative_impl_trait)]
02:49hiroI want to factor out ^
02:50emiliohiro: I think conservative_impl_trait is not stable yet, so I don't think we can do it
02:50emiliohiro: you'll need to create an iterator struct and all that
02:50hiroemilio: Oh, thanks. that's a pity..
02:50emiliohiro: yes :(
02:51hiroemilio: anyway, thanks for the quick answer!
02:51emiliohiro: no problem! :)
02:56jryansemilio: thanks for reviewing! :) I will look over it tomorrow
03:06waffles\o/ grid stuff merged finally!
03:06wafflesManishearth, thanks
03:06pcwaltonmy friend suggested i-tried for the name of my fast polygon triangulator
03:06pcwaltonI am not sure whether that is terrible or brilliant
03:07xidornheycam: how hard would it be to implement getDefaultComputedStyle in stylo?
03:07xidornemilio: ^
03:07heycamxidorn: not hard at all
03:07xidornheycam: ok, then let's implement it...
03:07emilioxidorn: yeah, i don't think it'd be super-hard
03:07emilioxidorn: "slightly annoying", I'd say :)
03:08heycamxidorn: yeah, seems like the reasonable thing to do at this point
03:08emiliowaffles++, thanks for carrying that over :)
03:09wafflesemilio, yeah, it's been like forever since I began that :p
03:09wafflesnox, only grid and grid-template remains, you picking something?
03:09wafflesnox, or no you don't, I'll take care of it :p
03:11emilioheycam: do you think it's reasonable to use a few of the remaining element state bits to store the implemented pseudo-element instead?
03:11heycamemilio: instead of ... where's it stored now?
03:11emilioheycam: a node property
03:12heycamemilio: how many pseudo element types do we have?
03:12heycamemilio: if there's enough space, then sure, we could, but I'm not sure how much space we have currently
03:12emilioheycam: I'm suggesting something like EventStates::InternalType mState: 48; CSSPseudoElementType mPseudoType
03:12emilioheycam: we definitely can
03:13xidornhmmm, backing out is a bit annoying as well
03:13heycamemilio: oh EventState
03:13* heycam thought element flags
03:14emilioheycam: yeah, we don't have enough flags, but I just noticed we have 16 bits of EventStates free
03:14emilioheycam: and we only need about 5 for pseudos I think
03:15heycamemilio: it's probably ok. might be a bit awkward just because of how we use the EventStates type currently
03:16emilioheycam: I just did something like this real quick, I don't think it changes the general usage of EventStates
03:16emilioheycam: ^
03:18heycamemilio: I'm worried more about the users of EventStates elsewhere, but maybe it's not a big deal
03:19emilioheycam: Oh, you mean people constructing EventStates from integers?
03:19emilioheycam: We already expose that constructor fwiw
03:20emilioheycam: whatever, I probably have some time to think about how good of an idea is before people put other 8 event state flags ;)
03:20heycamemilio: ok :-)
03:21* emilio goes to sleep for today
03:21emilioheycam: have a good day over there :)
03:21heycamemilio: thanks! I'll await more reviews from you tomorrow ;)
08:11noxcrowbot: tell jdm that I wonder why homu doesn't comment with test failures anymore.
08:11crowbotthere's a phone right next to you, but okay
08:18cynicaldevilnox: Free to answer some questions?
08:18noxcynicaldevil: Yes.
08:19cynicaldevilnox: What do you mean by a reflector for a dom object?
08:23noxcynicaldevil: You mean "you" as in Servo right?
08:23cynicaldevilnox: yes
08:23noxcynicaldevil: It's the actual JS object, owned by SpiderMonkey, that itself owns the Rust struct.
08:25cynicaldevilnox: So JS<T> and Root<T> are different type of pointers to the reflector object?
08:25noxcynicaldevil: Mmh, no, these point to the Rust-side of things,
08:25noxcynicaldevil: and the first field to which they point, transitively, is a Reflector.
08:26cynicaldevilnox: what do you mean by first field?
08:27noxcynicaldevil: Literally the first field.
08:27noxcynicaldevil: For example Node goes like this: struct Node { eventtarget: EventTarget, ... }
08:27noxand then struct EventTarget { reflector: Reflector, ... }
08:27cynicaldevilnox: Hmm, JS<T> has a single field, so I got confused
08:27cynicaldevilI got what you mean now
08:28noxcynicaldevil: JS<T> is a pointer, Root<T> is a pointer.
08:28noxFancy pointers, I concede, but pointers nonetheless.
08:30cynicaldevilnox: Do these types have any restrictions or can these be used just like any other type?
08:30noxcynicaldevil: They have plenty.
08:30noxcynicaldevil: JS<T> notably is forbidden on stack by unrooted_must_root,
08:30noxjust like any other #[must_root] type.
08:31cynicaldevilnox: yes, I was meaning to ask you about unrooted_must_root too.
08:31noxcynicaldevil: It&#39;s a poor man&#39;s actual check, but it works... For now.
08:31noxcynicaldevil: It&#39;s a lint that checks that #[must_root] are indeed transitively rooted.
08:32noxcynicaldevil: So you can&#39;t have a JS<T> local variable, but you can have a JS<T> field in a #[must_root] thing,
08:32noxbut that #[must_root] itself must be rooted somewhere, and it cannot either live on the stack directly.
08:33cynicaldevilnox: Wait, must_root types cannot live on the stack?
08:33noxcynicaldevil: Yeah, because JS must know about all #[must_root] values at all times.
08:33noxcynicaldevil: And obviously it can&#39;t know about stuff on the stack.
08:34cynicaldevilnox: So Root<T> is not a must_root type?
08:34noxcynicaldevil: Nope, because Root<T> roots.
08:34noxcynicaldevil: Root<T> is one of the escape hatches to actually manipulate values that must be rooted.
08:35noxcynicaldevil: You put #[must_root] things in Root<T> and then you can use them.
08:40noxcynicaldevil: Was that helpful?
08:40noxcynicaldevil: Did you read this?
08:40noxAnd this?
08:42cynicaldevilnox: Hmm, I&#39;d read the first page, but the second one&#39;s new to me. Thanks!
08:46noxManishearth: You need to update
08:46crowbotPR #16939: Use boolean instead of float to avoid nightly warning -
09:15* nox is trying to cleanup font longhands.
09:31noxShould I care about
09:33wafflesnox, nope
09:43noxwaffles: r? #16947
09:43crowbotPR #16947: Parse -moz-alt-content as a whole content value (fixes #15726) -
09:47wafflesnox, ... as if homu doesn&#39;t have enough PRs from you already :p
09:47wafflesoh, they all landed
09:47wafflesum, no there&#39;s 2
10:05heycamemilio: if you have time today to re-review my bug 1289868 patch, that would be great :) that (and bug 1364361) should unblock me from landing a bunch of patches
10:05firebot NEW, cam stylo: Use the result of CalcStyleDifference to cull parallel DOM traversal
10:05firebot ASSIGNED, cam stylo: AllChildrenIterator doesn&#39;t find NAC created by non-primary frames of elements
10:10noxIsn&#39;t a SystemFont supposed to set all shorthand properties?
10:20SimonSapinnox: macro_rules! generated with mako
10:21SimonSapin${sub_property.ident}: $${sub_property.ident}: expr
10:24wafflesooh nice
10:28SimonSapinnot the word I would have used, but
10:33wafflesSimonSapin, I&#39;m merely awing at the usage of Mako, I don&#39;t really like Mako-generated-macro-generated code
10:34SimonSapinah, that requires keeping the &quot;field&quot; order of the macro definition
10:34SimonSapinchange of plan
10:43noxSimonSapin: What?
10:44noxOh, that wasn&#39;t in reply to my SystemFont question ok.
10:44SimonSapinunrelated :)
10:50travis-ciServo failed to build with Rust nightly: CC nox, SimonSapin
10:50nox&quot;error: unused macro definition&quot;
11:15SimonSapinnox: you removing the unused macro or should I?
11:18noxI&#39;m not.
11:18noxSimonSapin: Can&#39;t remove it,
11:19noxSimonSapin: need to cfg it.
11:19SimonSapin#[cfg(feature = &quot;gecko&quot;)]
11:21SimonSapinnox: sigh
11:22SimonSapinthread &#39;rustc&#39; panicked at &#39;already have hash for FileMap(DefId { krate: CrateNum(14), node: DefIndex(0) => range/92375a1ff3a3ef6721ea553f45171916 }, &quot;/home/simon/servo1/ports/servo/<proc-macro source code>&quot;)&#39;, /checkout/src/librustc_incremental/persist/
11:22SimonSapinon incremental compilation, apparently
11:27crowbotIssue #42101: ICE: already have hash for FileMap(DefId { -
11:34SimonSapinnox: r? ^
12:19noxAh waffles had r+&#39;d it already.
12:28noxWhat&#39;s the purpose of ComputeDistance?
12:30noxSeems used only by nsIDOMWindowUtils.
12:30noxI kinda want to remove compute_distance and compute_distance_squared from the Animatable trait and put them in their own trait only compiled for gecko.
12:50est31nox: SimonSapin: the macro lint is a bit stupid regarding cfg-outed sections, but it cant really be fixed, not if the compiler choses to parse those sections
12:50est31which it does not do atm
12:50noxest31: What?
12:50noxest31: It was not cfg-outed.
12:50noxHence the warning.
12:50SimonSapinest31: working fine
12:51est31nox: yeah, but you can e.g. say that if the macro is used in some cfg-outed section, then it should me marked as &quot;used&quot;
12:51noxest31: Ah. No thanks.
12:51noxIn code paths where it&#39;s unused, I would like the compiler to tell me so.
12:52bzSimonSapin: where does the list of attrs that are case-insensitive in HTML live on the servo side?
12:52SimonSapinbz: selectors/
12:52SimonSapinbz: (to make a static hash set at compile time)
12:52bzaha; I wasn&#39;t seeing it in
12:53bzIn which version of selectors?
12:53SimonSapinmaybe it hasnt reached central yet?
12:54bzcentral is on 0.18
12:54SimonSapinbz: were not using it from anymore
12:54SimonSapinits in-tree
12:54bzwell, it&#39;s certainly not landed on m-c yet
12:54SimonSapinso the version number tend to not be updated
12:55SimonSapinwell, other servo crates have been at 0.0.1 since forever :)
12:55SimonSapinversions are not useful for path dependencies
12:56noxSimonSapin: Is selectors even still suitable for external use?
12:57crowbotjdm: nox said that I wonder why homu doesn&#39;t comment with test failures anymore.
12:57noxWith no releases made, maybe there were big changes that are too specific to Servo.
12:57SimonSapinnox: its still Very Generic
12:57jdmhigfive, you mean?
12:57SimonSapinwith a couple big traits and many associated types
12:57noxjdm: Yeah.
12:57noxSimonSapin: Ok.
13:07jdmnox: oh, it&#39;s because we introduced the filter-intermittents step
13:43noxjdm: I&#39;m not sure why the intermittent step means it can&#39;t print failures though.
13:43jdmnox: I explain in the issue that I filed about it
13:44noxjdm: Oops, sorry.
13:53GankroSo what&#39;s the relationship between reftests in gecko and servo? They seem structured very differently, directorywise?
13:54jdmthere is no relationship
13:54jdmwe rely on
13:54jdmgecko has their own reftest harness
13:55Gankrojdm: is wpt &quot;the future&quot; and gecko one day hopes to embrace it?
13:55jdmGankro: is the biggest blocker to that
13:55firebotBug 1263568 NEW, Make dbaron happier about wptrunner&#39;s reftest support
13:55jdmGankro: so is happening
13:55firebotBug 1363428 NEW, Implement a fast reftest backend for wptrunner
13:55jdmGankro: the biggest issue is the extra features that gecko&#39;s harness supports
13:56jdmand the effort required to port so many existing reftests
14:03noxManishearth: Around?
14:12GankroIs there a version of servo&#39;s --log-raw for an actual page? I&#39;m manually running some gecko reftests in Servo to figure out why they&#39;re getting different results out of webrender
14:27jdmGankro: you mean?
14:27jdmoh wait, no
14:27Gankrojdm: I mean I have a .html file, and I want to see what Servo is feeding webrender
14:28jdmGankro: wait, how would log-raw help with that?
14:28jdmGankro: I mean, you could take a screenshot with servo
14:28Gankrojdm: wow I&#39;m dumb
14:28Gankrotoo early in the morning :)
14:28jdmGankro: and run it through an image diff program
14:29jdm-o foo.png
14:30Gankrojdm: what I wanted was -Z wr-record :)
14:30GankroWhich works fine :)
14:30Gankroskimming my Browser Engine Dev Notes a bit too fast
14:35jdmcrowbot: what should I work on?
14:35crowbotjdm: profile the implementation of fullscreen api (
14:37xidornwhat&#39;s this...
14:37xidorncrowbot: what should I work on?
14:37crowbotxidorn: find a victim to own the high resolution time implementation (
14:42ajeffreyjgraham: ping re
14:42crowbotIssue #13994: [multi window] Constellation should be able to handle multiple root frame -
14:51noxGankro: I dig your phonebook job title.
14:51xidornwow, when was first checked in three months ago, there were over 600 lines. now it is less than 200 lines.
14:52noxxidorn: But if you had not coalesced related failures together, it would be even better.
14:52noxIt would be hundreds of line deletions, it would be glorious.
14:52noxThousands, even.
14:52Gankronox: ???
14:53noxGankro: What?
14:53noxGankro: &quot;Soundness Miner&quot;
14:53GankroI must have set that two years ago
14:53noxGankro: I understood you were on payroll only recently.
14:54Gankronox: internship
14:54noxI look up people on it to see if their meme game is on point.
14:54xidornnox: yeah the change of count should be even more impressive
14:55xidornbut I never try tracking that :/
14:55noxWeren&#39;t they stats at some point?
14:55noxI mean
14:55noxIs this still updated or not?
14:59xidornnox: I think I set auto sync with shinglyu&#39;s github repo
14:59xidornif that isn&#39;t updated then no
15:00xidornI always wanted to put sth useful there, but never got much time on that
15:01xidorns/on/for/ I guess...
15:03xidornif someone can give me some useful page for stylo, I&#39;m happy to put it on :)
15:19Gankrokvark: you may want to skim if you want to sit in on the allocator meeting
15:19crowbotPR #1974: Prepare global allocators for stabilization -
15:19kvarkthanks Gankro
15:28emilioxidorn: can you give me a test-case for bug 1364871? I&#39;d expect something like the following to work:
15:28firebot NEW, stylo: Restyle ::-moz-list-bullet and ::-moz-list-number pseudo-elements (for nsBulletFrame)
15:29emilioxidorn: (but it doesn&#39;t, so... i can dig into which properties apply to it and all that, but if you know off-hand that&#39;s easier :))
15:30emilioxidorn: oh, font-size seems to work, nvm
15:42SimonSapinjack, larsberg: has been running for 1 hrs, 45 mins, 46 secs. The builder has salt-minion taking 100% CPU and nothing else taking CPU.
15:42SimonSapinIm warry of the &quot;stop this build&quot; button, it has made things worse in the past sometimes
15:43jackSimonSapin: i think maybe restarting salt-minion?
15:43SimonSapinjack: how do I do that?
15:44jackservice salt-minion restart
15:45SimonSapinok, that build faild but things seem unstuck, a next one started
15:54jdmI have a suspicion that something is broken on with respect to using cargo on windows
15:54jdmour windows builder can&#39;t get new packages, and appveyor has been totally busted for the past 8 hours with the same error
15:57jdmacrichto: what do you make of ?
15:57crowbotPR #16905: Update harfbuzz-sys -
15:57jdmthe crate was published a few days ago:
16:02crowbotIssue #4072: cargo cannot download packages from on Windows 32 bit -
16:03jdmmmm, delight
16:03nicalnox: ping
16:04nicalnox: how do I tell bors-servo to try to re-land a change after it was rebased?
16:04noxnical: @bors-servo try
16:04noxAh misparsed you.
16:04noxnical: You r+ or r=someone it again.
16:06nicalnox: would retry do the same ? (it&#39;s already been reviewed but had to be rebased)
16:07noxnical: No, retry is for when CI died and the commits didn&#39;t change and you want to, well, retry.
16:07noxnical: Rebasing changes the commits, hence Homu forgets everything about past approvals on purpose.
16:08nicalok good to know
16:09* nical has always been snobbed by bors-servo up till now
16:10SimonSapinnox, emilio: can either of you review today? It would be nice to land it before I go on PTO :)
16:10crowbotPR #16954: Avoid returning / passing around a huge ParsedDeclaration type -
16:12SimonSapin&quot;error: unable to get packages from source&quot; :(
16:12SimonSapinsounds like what you were saying jdm
16:12jdmSimonSapin: yes
16:14SimonSapinjdm: could the builder be out of disk space?
16:14jdmSimonSapin: is this on windows?
16:14SimonSapinjdm: I dont know how to connect to windows builders
16:14jdmSimonSapin: no. I have a PR open that should fix it.
16:14SimonSapinoh ok
16:14crowbotPR #16953: Avoid the cert revocation check in Cargo. -
16:15jdmI want to see it fix appveyor before merging
16:22nicalnox: I am motivated to resurect all of my euclid work from last year and get it merged this time. If we can get it reviewed and landed by the end of my PTO (next week), I&#39;ll make a nice logo for euclid
16:22nicaland we can tag it 1.0
16:23noxnical: hype
16:26emilioSimonSapin: I can take a look
16:26SimonSapinemilio: thanks!
16:27emilioSimonSapin: before I go ahead, does it allow us to remove the hack of shorthands_without_all()? That&#39;d be quite nice :)
16:27SimonSapinemilio: I cant find that
16:28SimonSapinemilio: but theres still a hack that sounds like that
16:28emilioSimonSapin: shorthands_except_all, sorry
16:28emilioSimonSapin: heycam did that so it didn&#39;t blow the stack :)
16:28SimonSapinemilio: I didnt touch that, let me look
16:31SimonSapinemilio: I dont see how to remove that, I even have one more thing that could use it: [PropertyDeclaration; MAX_SUB_PROPERTIES_PER_SHORTHAND_EXCEPT_ALL]
16:31SimonSapin ${max(len(s.sub_properties) for s in data.shorthands if != &quot;all&quot;)};
16:32emilioSimonSapin: Oh well. Yeah, I thought it would be hard to remove, so nevermind :)
16:37crowbotIssue #4072: cargo cannot download packages from on Windows 32 bit -
16:37jdmyep, lars pointed me that way. thanks!
16:43emilioSimonSapin: r=me
16:46SimonSapinemilio: Im not sure what to add to the SourcePropertyDeclaration doc-comment
16:46SimonSapinemilio: it already talks about parsing one `key: value`
16:47emilioSimonSapin: fair enough, I guess :)
16:56kvarkany WR reviewers here to look at ? considering gw is on the weekend already
16:56crowbotPR #1278: Added Renderer::read_pixels_into -
16:58Manishearthemilio: there?
16:58emilioManishearth: yup
16:58Manishearthemilio: do you know the current perf status?
16:59emiliokvark: I can try to take a look later, though I&#39;ll delegate if I&#39;m not confident enough :)
16:59emilioManishearth: perf status as in...?
16:59kvarksure thing, thanks emilio
16:59Manishearthemilio: hold on, better in PM, I can provide context :)
16:59emilioManishearth: sure
17:04Manishearthbholley_mobile: ping whenever you&#39;re around
17:12emiliokvark: oh well, was easy enough :P
17:32Manishearththat bors queue isn&#39;t getting any better
17:33Manishearthbut I see different PRs in it. are we just working too hard?
17:34mbrubeckManishearth: review ping for ? maybe if I get it in the queue today it&#39;ll land by the end of the weekend :)
17:34firebotBug 1365370 ASSIGNED, stylo: Use correct counts when copying from image layers
17:34Manishearthmbrubeck: i thought i r+d that
17:34Manishearthoh wait
17:34Manishearthit is different
17:34jryansemilio: review comments on visited seem logical to me, i&#39;ll try splitting outputs to MatchingResults like you mention. i agree the copying inputs / outputs is not the best! (i only added it while rebasing after bloom appeared) :)
17:35mbrubeckManishearth: this was some code cleanup I noticed when working on the other one
17:35Manishearthmbrubeck: fwiw such things work best as direct servo side PRs
17:35mbrubeckManishearth: okay
17:37kvarkemilio: thanks a bunch!
17:42jryansemilio: oh, but should MatchingResults with relations, etc. be a new out param or return value? things like `matches_simple_selector` already have a natural bool return values...
18:20* jdm -> out for a while
18:26emiliojryans: I mean a return value of `match_{primary,pseudos}
18:27emiliojryans: (selectors doesn&#39;t know about rule nodes)
18:28jryansemilio: okay, so matching context fields as is with in / outs together, but then extract the outputs plus rule node to return from match_primary, etc.?
18:30emiliojryans: yeah, I think that&#39;d be the nicest option so far, but if you have other alternatives I&#39;d be glad to hear
18:30jryansemilio: i think it makes sense, thanks!
18:47Manishearthbholley: what was the problem with making it slow?
18:48bholleyManishearth: I am filing a bug now
18:48bholleyManishearth: also, you pinged me earlier?
18:48Manishearthcc pls :)
18:48Manishearthbholley: to ask you about the perf thing. but then i realized you were already ccd on that thread :)
18:54* bholley Manishearth: bug 1366347
18:54firebot NEW, stylo: Rewrite
18:57Manishearthbholley: Dumb Things ftw
18:58jryansbholley: sounds like a great improvement!
18:58bholleyjryans: yeah, sure hope so - just cleaning up the patch before pushing to try
19:07mbrubeckbholley: Interesting! I notice that Servo `layout::parallel` uses the same chunking as the current `style::parallel`. I wonder if it&#39;s a better fit for layout, or similarly bad?
19:07bholleymbrubeck: I wouldn&#39;t be surprised if it were similarly bad
19:08bholleymbrubeck: I remember larsberg sent me some research results from somebody who suggested that the parallel traversal was only a win for doms with thousands of elements
19:08bholleymbrubeck: and this might explain why
19:08Manishearthbholley: pcwalton rebutted that research at some point IIRC
19:08mbrubeckbholley: Also I wonder if we can use ParallelIterator or `rayon::join` instead of `Scope::spawn`... I guess I should wait to see what your patch changes.
19:09Manishearththey were not measuring the right things or something
19:09bholleymbrubeck: we can&#39;t
19:09Manishearthi don&#39;t recall
19:09mbrubeckbholley: Because of the borrowing?
19:09Manishearthregadless, great work on :)
19:09bholleymbrubeck: no, it just doesn&#39;t let us tune our parallelism in the right way
19:09bholleymbrubeck: (in a way we do in this patch)
19:09bholleymbrubeck: basically, the issue is
19:09bholleymbrubeck: parallel iterators and join use tail calls
19:10bholleymbrubeck: but that means that we&#39;d need to finish processing all the siblings and queuing up all their children before we make any of the children available for stealing
19:10bholleymbrubeck: the current code does that but it&#39;s dumb
19:10bholleymbrubeck: we should make the children of the first sibling available for stealing as soon as we finish styling that first sibling
19:24larsbergManishearth: IIRC there were questions for rzambre but there was no rebuttal or request to change the measuring method, etc.
19:24larsbergManishearth: that line of work continues apace, with more machine learning + some really nifty online learning ideas (though a bit on pause this summer, due to internships)
19:25larsberge.g., one flaw is that it doesn&#39;t account for current machine load and the learning is done offline on a dedicated machine
19:29Manishearthlarsberg: yeah
19:29Manishearthlarsberg: idk pcwalton mentioned a bunch of issues a few months ago
19:38SimonSapinlarsberg: has been running for suspiciously long, the log is not making progress, and SSHing into that builder asks me for a password
19:39SimonSapinlarsberg: I can SSH into but not
19:40ManishearthSimonSapin: killed the build
19:41SimonSapinManishearth: you did? It looks unchanged
19:43ManishearthSimonSapin: its not dying
19:43ManishearthSimonSapin: i suspect the buildslave is dead
19:44SimonSapinManishearth: SSH is responding, but asking for a password
19:44SimonSapinrejecting my key
19:45Manishearthdoing a resalt
19:45Manishearthyep, minion down
19:53bzbholley: funtimes, on the thing
19:53bholleybz: yeah
19:54bzbholley: I still need to do more talos profiling
19:54* bz has been stuck in review and spec-review land
19:54bholleybz: so
19:54bholleybz: my general take
19:54bholleybz: is that the thing should help a lot
19:55bholleybz: and then I _think_ the problem is that we&#39;re getting killed on lots of tiny traversals
19:55bzbholley: That seems plausible, if talos is running on multicore hardware
19:55bzbholley: the profiling we were doing was with STYLO_THREADS=1, right?
19:55bholleybz: so, I just landed that patch to make us not use parallel stuff for the tiny traversal
19:55bholleybz: but
19:55bzright, I saw that
19:55bholleybz: we still do a lot of stuff even to do them sequentially
19:55bholleybz: heap-allocating and zeroing the bloom filter, for example
19:56bholleybz: there&#39;s just a whole lot of stuff
19:56bholleybz: so first, I want to make that stuff cheaper
19:56bholleybz: and then, if it&#39;s still too expensive, potentially add a &quot;non-traversal&quot; path into the style code
19:57bzthe bloom filter thing is no worse than gecko, right?
19:57bzJust to make sure
19:58bholleybillm: I think it is worse
19:58bzLanding, so have to go offline for a bit
19:58bholleybz: because in gecko, we build it once and pass it down the frame constructor
19:58bzsomething laptops, put away, something
19:58bholleybz: in stylo, _each_ tiny traversal triggered by the fc
19:58bholleybz: has to build a new bloom
19:58bzbholley: oh, ick
19:58bholleybz: I will fix it
19:59bzback later, but not sure when
19:59bzPossibly &quot;Sunday night&quot;....
20:15larsbergSimonSapin: Manishearth: edunham: yeah, on servo-mac3 my guess would be that it got hit with a forced OS update and it reset some of the mumbo-jumbo allowing remote access & remote desktop. That happens shockingly often to minis :-/
20:15larsbergManishearth: Thanksf or doing the re-salt, let us know if it&#39;s not working!
20:15larsbergManishearth: emily is kicking over a bunch of new minis to increase the mac pool size :-)
20:15larsbergcc bz ^
20:15SimonSapinlarsberg: how is the size of the rest of the pool
20:17larsbergSimonSapin: is the ones that are connected
20:17larsbergcounting &quot;actual&quot; machines is a little tougher as they&#39;re only visible by unifying the macstadium view and the EC2 view
20:18larsbergedunham: we don&#39; t have anything public that lists all the h/w, right?
20:18edunhamlarsberg: afaik nope
20:18aneeshusalarsberg: edunham: you could try salt &#39;*&#39; ?
20:18edunham has the slave IPs
20:18edunhamor at least it used to
20:18larsbergooo good point aneeshusa
20:20Manishearthlarsberg: it&#39;s not working
20:20Manishearthlarsberg: the server is down and we need to kick it via aws
20:20Manishearthi can&#39;t do that
20:21Manishearthlarsberg: ?
20:25larsbergManishearth: edunham: Do you think we should hit the big macstadium reboot button?
20:26* larsberg has had bad luck with that button in the past
20:29SimonSapinturn all the things off and on again
20:29aneeshusaManishearth: larsberg: I can servo-mac3 from salt OK, we can try salt &#39;servo-mac3&#39; system.reboot
20:30larsberghrm, I was able to ssh into it, too
20:30larsbergmaybe just Manishearth is broken :-)
20:44KiChjangweird, i can&#39;t get rust-mozjs to build on windows
20:54Manishearthlarsberg: or perhaps the highstate succeeded :)
20:54Manishearthjust delayed
20:55emiliobholley: I really really thought you were aware of the sibling-children-coalescing thing. That was in my backlog of stuff I wanted to look at, but glad you rewrote it all :P
20:59jdmeveryone make a list of their assumptions right now
20:59jdmlet&#39;s get this cleared up
21:14bholleyemilio: I must have been
21:14bholleyemilio: I vaguely remember talking about it in September in Taipei
21:14bholleyemilio: but I thought we&#39;d fixed it
21:14emiliobholley: I&#39;m pretty sure I talked with you about it, yeah
21:15emiliobholley: anyway, glad to see parallel perf is way better with your patches now :)