mozilla :: #servo

9 Oct 2017
00:23mbrubeckjwilm: The "--device-pixel-ratio" flag is the only way to set it as far as I know, but it's a straightforward mapping where a ratio of 1.0 (1 dppx) == 96dpi
00:23mbrubeckjwilm: We don't look at the actual hardware DPI though
00:23mbrubecksimply because we haven't yet written any code to do so
00:23mbrubeck(at least on Linux; I'm not sure about other platforms)
00:29jwilmmbrubeck: interesting; hadn't heard about 96dpi being considered a dpr of 1
00:30mbrubeckjwilm: You can think of DPR as being "dots (device pixels) per px"
00:30mbrubeckjwilm: and in CSS, 1in == 96px by definition
00:31jwilminteresting; so for correct rendering, you really would want to be using the display's DPI rather than a system configured scale factor
00:34mbrubeckjwilm: Yes, *if* you want 1 CSS px to be as close as possible to 1 physical inch
00:35mbrubeckI mean 1/96"
00:35mbrubeckjwilm: But px (and units like "in" that are based on it) are actually expected to vary in physical size with factors like viewing distance
00:36mbrubecke.g. a 16px font should not be 1/6" tall when it's on a giant projection screen a hundred meters away
00:36jwilmWouldn't that be captured by a simple scale factor?
00:37jwilmso wait a sec, do you care about the device-pixel-ratio for that case or something like "zoom"
00:37mbrubeckGenerally for desktop systems the default DPR (at "100%" zoom) is based on the physical DPI of the monitor, though usually rounded somewhat
00:37mbrubecksince images and such look better at exactly 2.0 dppx than weird numbers like 1.834 dppx
00:37mbrubeckApple desktops always use 1.0 or 2.0 dppx
00:37mbrubeckby default
00:38mbrubeck(but users can configure other settings)
00:39mbrubeckAndroid and Windows have a larger set of default ratios, but still a limited set
00:40jwilmIt sounds like DPI isn't all that important then; just DPR (with 1.0 being 96 DPI)
00:42mbrubeckIn mobile browsers it gets more complicated because pinch zoom causes the DPR to vary continuously...
00:42mbrubeckServo currently has a somewhat simplistic view of things
00:42mbrubeck explains some of it
00:43mbrubeckand the other "pixel" unit types in
01:03jwilmmbrubeck: thanks for all your insights
02:33WindowsBunnymbrubeck: Windows actually allows a practically continuous range of DPI values between 1x and some really high DPI
02:34mbrubeckWindowsBunny: Oh yeah. Most of my experience is from the Windows 8 "Metro" environment where at least at first, only a few configurations were allowed, IIRC. But that never applied to Win32.
02:35WindowsBunnymbrubeck: Sure, the normal settings menu has a limited selection, but there is an advanced dialog that lets you select an arbitrary value
02:39larsbergDuring the longhorn / win7 days we had a crazy array of devices and no idea of what the final high DPI device support / ratios would look like. Ironically, it was all *desktop* (monitor) prototypes, and almost nothing in our tabletpc formfactor.
02:48WindowsBunnyThe solution is to simply make your code able to cope with *any* DPI
02:49larsbergDown that road lies autolayout, and autolayout is the golden path to destruction
03:34heycamxidorn: do you want to implement the :scope stuff. I guess it's not too hard.
03:34xidornheycam: yeah, I can do that. it doesn't sound very hard to me either :)
03:36heycamI guess I forgot about :scope's effect on matches etc.
03:36xidornI wasn't aware of that either...
06:53heycamhiro: hooray, you can reproduce in your local build! :-) will you look into that?
06:53hiroheycam: I am looking now, no clue yet.
06:55heycamhiro: the things I wanted to check were -- did the style sheet parse correctly? did it get added to the XBL stylist?
06:55hiroheycam: OK, I will check it. which is the style sheet exactly?
06:56heycamhiro: chrome://global/content/bindings/videocontrols.css
06:56hiroheycam: OK, where should I set a break point to check it?
06:57heycamhiro: we should load it from nsXBLResourceLoader::LoadResources
06:57heycamand I guess you can try reverting emilio's patch, or part of it
06:58hiroheycam: ah, I can't probably check it correctly because I got an assertion on debug build. this assertion bug 1339009
06:58firebot NEW, Assertion failure: clip (Need to be clipped wrt aASR. Do not call this function with an ASR that our
06:58hiroI might need to rebuild without the assertion
06:58heycamoh I haven't seen that one for a couple of months
06:59* heycam brb
06:59hiroheycam: the assertion happens repeatedly when opening the html5video.
07:02emiliohiro: yay for being able to repro it!
07:02hiroemilio: great!
07:03emiliohiro: there's a comment saying what may change in the suspect patch, not sure if you read it
07:07hiroemilio: I have already read it, but I do not understand it yet.
07:07hirohmm, I can't reproduce on the content while attaching gdb.
07:08hirogaa, the assertion.. I need to rebuild.
07:09emiliohiro: so, before my patch, we created a `MatchingContext` without bloom filter, nth-index cache, and all that
07:09emilioxidorn: ping?
07:10emilioxidorn: is the scope pseudo-class supposed to be used in the dynamic profile?
07:11xidornemilio: yes
07:12xidornemilio: but without <style scoped> that is basically :root in dynamic profile
07:12emilioxidorn: yeah, I see
07:23emilioheycam: r? ^
07:25heycamemilio: looking now :-)
07:35emilioxidorn: thanks for looking at bug 1406562 btw, hadn&#39;t still gone through it
07:35firebot ASSIGNED, stylo: Assertion failure: mOwner == ExpectedOwnerForChild(aFrame) (Missed some frame in the hierarch
07:45emiliohiro: ping?
08:31hiroemilio: pong. sorry was cooking.
08:32emiliohiro: no worries, was going to investigate the unstyled video thing and wanted to ask whether you had found something interesting or not, in order to avoid duplicating work
08:33hiroemilio: that&#39;s good to hear. I did find nothing yet.
08:34emiliohiro: no worries, thanks for checking!
08:50emilioheycam: doesn&#39;t Arc::eq do that already?
08:51heycamemilio: does it?
08:51emilioheycam: huh, seems it doesn&#39;t... should it? :)
08:51heycamemilio: um, maybe!
08:51heycamemilio: rather I do that?
08:52heycamthough I don&#39;t know if that&#39;s surprising thing or not
08:52emilioheycam: how could it be?
08:52heycamemilio: yeah, suppose not. surprising if it&#39;s quicker? :)
08:53* heycam updates the patch
08:53emilioheycam: hah, I just figured out the unstyled video thing
08:53emilioheycam: I&#39;m baffled
08:55emilioheycam: how much do you know about XBL and quirks mode? :)
08:56heycamemilio: will until I now I thought they were pretty orthogonal things...
08:57emilioheycam: so did I! but if I hackily swap `self.quirks_mode` and use the XBL stylist&#39;s claimed quirks mode instead (need to know where does it come from, maybe it&#39;s always NoQuirks?) The bug goes away...
08:59heycamwe&#39;re looking up the wrong tables when styling then or something?
08:59emilioheycam: I&#39;m figuring out why, my idea when doing the change was &quot;well, if it&#39;s in quirks mode we may look case-insensitively or what not, but it&#39;s not incorrect&quot;
08:59emilioheycam: but turns out it is
09:00heycamI wonder if the fact that quirks mode gets set async on a document is why it&#39;s a bit random
09:00heycamon the audio one at least
09:01heycam(emilio: updated the ptr_eq thing)
09:02emilioheycam: nope, I see what&#39;s going on now
09:03emilioheycam: when we insert on a quirks mode document, we key on the lowercase atom
09:04emilioheycam: so looking up with a `Quirks` matching mode on a stylist that was filed with `NoQuirks` makes us look with the wrong keys
09:04emilioheycam: let me try to build a test for it
09:04heycamemilio: so we just find no rules
09:04heycamwhen looking up
09:04emilioheycam: right
09:04heycammakes sense :)
09:05heycamare the top level media documents quirks mode?
09:05emilioheycam: yeah, it always does in the end
09:05emilioheycam: it seems so
09:06emilioheycam: how do I create a fully quirks-mode doc? Omitting doctype gives me almost-standards, right?
09:07heycamemilio: actually I don&#39;t remember the way to trigger those differently
09:09heycamso I guess no doctype means actual quirks mode
09:09heycamalmost standards is HTML 4 / XHTML 1 DTDs
09:12emilioheycam: hmm, I&#39;d expect something like this to display undisplayed controls:
09:12emilioheycam: but it clearly doesn&#39;t
09:15heycamemilio: maybe the binding needs to apply before we run the quirks mode setting runnable
09:15heycamI&#39;m not sure
09:15heycam(well, not runnable it seems, but the function that does it)
09:29emilioheycam: r=me on the servo_arc thing
09:30emilioheycam: I have tried a couple things and haven&#39;t managed to get it to work, I think the binding needs to get reused somehow
09:31heycamemilio: ok. then i wouldn&#39;t spend too much time trying to get a test written
09:32emilioheycam: r? ^
09:32emilioheycam: I&#39;ll file a bug, we should at least be consistent about which mode to use for styling I think
09:33sewardjglandium: ping
09:34heycamemilio: looks good, r+
09:34emilioheycam: thanks!
09:35heycamemilio: I&#39;m almost out of time today now, and haven&#39;t had luck with the avenues I&#39;ve been trying with the custom properties stuff. if you have time do you want to profile with and without the changes I just reviewed?
09:35* heycam had some trouble cherry picking locally to before bobby&#39;s hash table journaling patches
09:35emilioheycam: sure, I&#39;ll do that
09:35heycamemilio: great. I am interested to see if the patterns in the page are helped by it
09:36emilioheycam: yeah, I just reverted the hashmap in OrderMap instead to be non-Diagnostic
09:36heycamemilio: STR would be to have a youtube account with many subscriptions in it, then visit the subscriptions page. I&#39;ve been profiling just a reload of the page until it settles down
09:36emilioheycam: cool
09:36* emilio finds some cool channels to subscribe to :P
09:37heycamI just kept clicking Subscribe in the suggested box on the top right until I reached some temporary limit of 75 channels
09:37heycamemilio: (it&#39;s a a public holiday here tomorrow so I probably won&#39;t get time to continue investigating much before wednesday)
09:39emilioheycam: cool no worries. I have other bugs to look at but they&#39;re probably not as prioritary
09:40heycamemilio: whatever further optimizations we do for the custom properties, I think it has to take into account the silly rule in the page that provides like 80 specified custom properties on every element :/
09:40emilioheycam: oh, fun stuff
09:41emilioheycam: my patch should help with it I think, at least perf-wise. We could try optimize memory-wise too I guess
09:41heycamemilio: memory wise would be interesting (and yeah I also thought that we must be using a bunch of memory here), but less important I guess
11:29noxManishearth: Ping.
11:36Manishearthnox pong
11:36noxManishearth: This comment: how does it relate to #11722?
11:36crowbotIssue #11722: Create a central repository of Origins that can be trusted -
11:37noxThe Blob URL store is per-global, no?
11:38Manishearthcould you shorten and paste?
11:38Manishearthcant open large links from phone
11:38Manishearthwithout effort
11:41Manishearthnox: no idea
11:42noxThe whole thing is a mess anyway, and I need to extend the concept to MediaSource.
11:42Manishearthi dont know what &quot;second field&quot; relates to there
11:42Manishearthi presume it has something to do with making blob urls unguessable and unforgeable
11:43noxManishearth: The format of blob URLs is mandated by spec, which is why that comment confuses me.
11:47noxI&#39;m not even sure the blob URL store should live in ScriptThread.
12:08jntrnrnox: why are you trying to resurrect is it needed for something else?
12:09noxjntrnr: No, Im assigned to it and did triaging.
12:09noxWe dont need a reason to try to carry our contributors patches, AFAIK.
12:09eijebongnox: Final PR will be 125 files changed, 1183 insertions(+), 1205 deletions(-) (still have some dependencies depenfing on bitflags 0.x though, but servo itself is done :))
12:10noxeijebong: Nice.
12:10eijebongGoing to read it once before submitting (I think it can be merged as is and I&#39;ll update dependencies after
12:11jntrnrnox: what I mean is, does it help in some way? I thought we were moving away from glutin
12:12noxjntrnr: It should fix #12616 and #15240.
12:12crowbotIssue #15240: Handle KeyEvent with no virtual key code. -
12:12crowbotIssue #12616: Can&#39;t enter <#+ keys on german keyboard -
12:13jntrnrk, will take a look. So when I said &quot;is it needed for something&quot; the answer is yes :)
12:13noxWell thats written in the PR description.
12:14jntrnrnox: it isn&#39;t. It says &quot;These PR is not completed yet&quot; and no link to bugs it fixes, which is why I asked
12:15noxIts written in the PR description.
12:15jntrnroh I see, it&#39;s after the line... it&#39;s early here :p
12:16* jntrnr has to dust off his Servo codebase knowledge
12:16noxI just don&#39;t understand the Windows-specific code that landed after that PR was made.
12:16jntrnrnox: it&#39;s gross. I wish we could pull it out
12:16nox(And I don&#39;t have Windows to begin with.)
12:16jntrnrWindows uses different keyboard event ordering than Linux/macOS
12:16jntrnrand we have to mimic it
13:59mrobinsonUgh. Is the WPT test count broken again? :(
14:16Gankrobholley_mobile: you got a hit!
14:50jdmmrobinson: probably related to the update to wptrunner that occurred last week
14:50jdmGankro: crash report, or something else?
14:51GankroI don&#39;t think I have perms to get the juicy log of the hashmap ops tho ;_;
14:51mrobinsonjdm: No problem. Did it have an update and then get reverted at some point?
14:51jdmmrobinson: no, nothing should have been reverted
14:52jdmmrobinson: my point is merely that updating wptrunner may have changed the data structures that the formatter tries to interact with
14:53mrobinsonjdm: Hrm. Perhaps the revert was in wptrunner then. The data structure seemed to change and then change back. And now it looks like it may have changed again.
15:02crowbotPR #44917: Immovable types prototype where the Move trait is builtin -
15:03noxajeffrey: Lol @ parsell in the list of the cargobomb failures.
15:03ajeffreynox: ooh
15:04ajeffreyAnd yes, I noticed parsell,
15:04ajeffreyNot too surprising that it&#39;s stretching the type system more than most!
15:08ajeffreynox: looking at the issues that you pinged me about...
15:09ajeffreyIt seems like we use &quot;assigned&quot; for different things,
15:09ajeffreyI use it for &quot;put this on my queue to deal with sometime&quot;
15:10noxajeffrey: Assigned conveys to people that you are working on it.
15:10noxAnd that other people probably shouldn&#39;t bother looking at it.
15:10ajeffreyIt looks like you use it for active working.
15:10noxajeffrey: Seems like you want a todo.txt file on your computer.
15:10noxajeffrey: Especially for the most succinct issues that don&#39;t make sense to anyone but you.
15:10ajeffreyI don&#39;t mind which semantics, as long as we all agree!
15:11ajeffreynox: I do use GitHub issues as my to do list.
15:11noxajeffrey: I disagree with the use of GH as a personal to-do list.
15:13noxSeeing a high issue count is a bit tiring, and I would like to not artificially inflate it.
15:14ajeffreyThe &quot;assigned&quot; flash won&#39;t help with the issue count, at least not directly.
15:15noxajeffrey: Can&#39;t be assigned if the issue doesn&#39;t exist.
15:16noxajeffrey: Though that&#39;s why I also suggested making some of them E-Easy ones.
15:16ajeffreyI&#39;ll have a look to see which ones are E-easy.
15:17noxajeffrey: Low priority, I also made these comments because I was triaging anyway.
15:18ajeffreyDoes anyone else have an opinion about whether &quot;assigned&quot; should only be used for issues people are actively working on? jack?
15:19cynicaldevilI can&#39;t find any mention of an XML parsing algorithm in the spec. Is it the same as the HTML one?
15:19noxajeffrey: I&#39;ve noticed that some of the issues I&#39;ve filed and assigned to me months ago, some of them I don&#39;t even know why I filed them.
15:19cynicaldevilnox: ^
15:19noxcynicaldevil: Nope, unspecified.
15:20cynicaldevilnox: How was it implemented, then?
15:20ajeffreynox: yes, future me often regrets how terse past me was :/
15:20noxajeffrey: Yeah, that happens. :)
15:20jackajeffrey: that is mostly supposed to be how it works
15:21jackC-assigned is definitely supposed to work that way. assigned sometimes gets used as a way to ask someone to look at an issue
15:21ajeffreyjack: OK, I&#39;ll unassign myself from the issues I&#39;m not actively working on then.
15:22lqdajeffrey nox at the time those GH issues didn&#39;t exist I did the only couple &quot;E-easy&quot; that were hidden in the documentation back then :D
15:23cynicaldevilnox: Ok, I&#39;m supposed to be reviewing the PR to remove the complete_script method from x5e, and I remember you saying that some work needed to be done on it.
15:23noxThis triage made me notice that often when we have tickets with questions as titles, we never end up answering the damn question.
15:24jdmmake a bot that yells at anybody who files an issue with a question
15:24noxjdm: Works for &quot;Consider blabla&quot; too.
15:24jdmactually, we could add a label for &quot;open question&quot;
15:24jdmand have the new highfive bot automatically bother people after a week
15:25cynicaldevilnox: I wanted to know what &#39;other stuff&#39; you were referring to.
15:26noxcynicaldevil: Making x5e not own the input at all.
15:26noxcynicaldevil: Just like h5e.
15:31cynicaldevilnox: Then just removing this method: and all calls to it would do the job?
15:46noxajeffrey: There should be a per-issue star feature on GH,
15:46noxajeffrey: so that you can still keep track of whatever you want.
15:47jdmwe could each make our own labels
15:49ajeffreynox: yes, it would be nice to mark issues as &quot;must get round to looking at this&quot;.
15:49ajeffreyjdm: wouldn&#39;t that need a label per contributor?
15:49jdmajeffrey: yes, I was not actually making a serious suggestion
15:50ajeffreyjdm: ah, this is an example of your earthling &quot;humour&quot;.
15:50jdmI mean, to be fair, it sounded plausible and I gave no textual indication of sarcasm
15:51noxajeffrey: The full text search is quite ok.
15:51noxajeffrey: So technically, you could choose a unique enough word to put in such issues.
15:52* nox claims the word baguette.
15:54jdmcrowbot: what does the web need?
15:54crowbotjdm: the web assembly working group is popular in the IoT space (
16:08ajeffreycrowbot: I actually believe that one. Have a botsnack!
16:08* crowbot beams
16:08ajeffreystandups: following up on nox&#39;s epic bug triage
16:08standupsOk, submitted #51784 for
16:11noxajeffrey: &quot;general complaining&quot; made my day.
16:12ajeffreynox: glad to be of service
16:16larsbergone pretty cool alternative to see what people are *actually* working on in GH issues is
16:16noxlarsberg: Now on sick leave from PTSD because this looks too much like a scrum method.
16:16larsberg(layers on top of the existing GH issues w/ some extra dat)
16:18larsbergnox: please open an issue for your sick leave in the backlog for consideration next sprint
16:18noxlarsberg: Ah ah ah ah.
16:20larsbergbut srsly it&#39;s just a basic kanban board. though certainly you could use the tool for scrum, much like you could choose to use a bagel as a hat.
16:20noxlarsberg: Kanban?!
16:21* nox reports larsberg.
16:22noxcynicaldevil: This must disappear, just like it already did in h5e.
16:42noxajeffrey: We could use projects too.
16:43noxajeffrey: &quot;Stuff Alan cares about&quot;, &quot;Stuff nox cares about&quot;, etc.
17:09noxSimonSapin: Making a PR to try thinlto on CI.
17:10SimonSapinnox: as an additional build to make sure it doesnt break?
17:10noxSimonSapin: No, just pushing some code and doing @bors-servo try
17:28SimonSapinbz: what do you think of landing error message column numbers with a 2~4% parsing perf regression?
17:28firebotBug 1378861 REOPENED, stylo: Error columns reported by Stylo differ from Gecko&#39;s parser
17:35cynicaldevilnice branch name.
17:36emilioSimonSapin: r? ^
17:37emilionox: doesn&#39;t that PR remove all other `RUSTFLAGS`
17:37emilionox: (which may also affect the build, though I don&#39;t know if we use them actually)
17:37noxAre there others?
17:38noxOh I see.
17:38noxI don&#39;t want to interrupt the try, ugh.
17:38noxi&#39;ll fix that later.
17:38noxIt&#39;s not like it could be merged as is anyway.
17:41SimonSapinemilio: is that the same code that rewrites?
17:42SimonSapin(which Im very late reviewing, sorry xidorn)
17:43emilioSimonSapin: I think so, but that would need a pretty big rewrite after #18791
17:43crowbotPR #18791: style: Optimize custom property cascading. -
17:43emilioSimonSapin: plus, I think we can actually implement cycle removal while doing variable substitution on custom properties
17:44emilioSimonSapin: (keeping track of the edges of the dependency graph while traversing it)
17:44emilioSimonSapin: I&#39;ll talk with xidorn to see what he thinks about it
17:44SimonSapinanyway I should probably do the older review first, yall figure out how to land them :)
17:45emilioSimonSapin: meanwhile that patch gives a pretty nice perf win for free
17:45SimonSapinemilio: do you have an opinion on landing with a perf regression?
17:45firebotBug 1378861 REOPENED, stylo: Error columns reported by Stylo differ from Gecko&#39;s parser
17:46emilioSimonSapin: are the lines reported different, or incorrect?
17:46emilioSimonSapin: if it&#39;s just different, I&#39;d prefer not to land it, actually. But if it&#39;s a matter of correctness, I guess a small perf regression may be acceptable
17:47SimonSapinemilio: currently the location reported is always the start of e.g. a declaration, with this patch its more precise inside the value
17:47SimonSapinemilio: this is for devtools console messages, it doesnt affect content
17:48emilioSimonSapin: doesn&#39;t look terribly important to me, tbh, specially if we print out the declaration... But I guess tromey or jryans may have different opinions about the importance of this
17:49jryansSimonSapin: emilio: i have a hard time judging the important since we don&#39;t know how many people even see these messages
17:49tromeyI think this was a request coming from bz who found the current locations not-great
17:50jryanstromey: yes, i believe that&#39;s accurate
17:50tromeyyes, I don&#39;t really know why the console filters the css warnings by default
17:50tromeythat&#39;s confused me a few times
17:50jryanstromey: i think it&#39;s because most sites generate _many_ of them, so it would hide other things
17:51emiliojryans: tromey: SimonSapin: Then I think I&#39;ll defer to bz, since he usually also cares a lot about perf too... :-)
17:51jryanstromey: but it&#39;s tricky, since they filter bar is also hidden by default, so it leads me to suspect people don&#39;t even know these is an option to show them
17:51emiliojryans: tromey: Is there any way to somehow avoid reporting them when the terminal isn&#39;t open? It&#39;s kind of a waste, and affects parser perf heavily
17:51* jryans seems to type too fast, need to proof read more :S
17:52tromeyI think there&#39;s no way to do that
17:53SimonSapinemilio: my branch is all about including the location of part of Result::Err, its not something we can easily turn off
17:53emiliotromey: that&#39;s pretty unfortunate. Those error messages create very heavy-weight XPCOM objects
17:53tromeythe platform console code stores all these things so you can open the console and see what happened after the fact
17:53emilioSimonSapin: I know, I&#39;m talking about the higher-level reporting stuff
17:53emilioSimonSapin: that decreased parser perf by a lot back when jdm landed it:(
17:54emiliotromey: I know, I know... I just think it&#39;d be nice, since this code happens to be pretty hot
17:54emiliotromey: maybe reparsing sheets with it enabled is good enough?
17:54emiliotromey: IDK
17:54emiliotromey: it&#39;s just unfortunate having to pay for it all the time
17:54tromeyI&#39;m not sure who would own a decision like that
17:56SimonSapinnox: syn fails to build with thinlto apparently
17:56crowbotIssue #45131: LLVM assertion with 16 codegen units and ThinLTO -
17:56noxBuilt fine for me so maybe my patch is crap.
17:57SimonSapinmaybe only on some platforms, or with some set of features?
17:58bzWe really need a general solution for &quot;things that are nice to have when debugging but expensive to collect all the time&quot;
17:58bzCSS errors
17:58bzasync stacks
17:59bzThat sort of thing
17:59tromeyin the future, maybe whether the devtools system addon is installed
17:59tromeythe issue there though is that the dev edition will appear to be slower
18:00bzAnd all the Gecko developers will always be profiling with it installed, presumably, etc....
18:00jryansbz: each one of these features may have different (assumed) requirements about whether it works without reload, etc.
18:00bzI mean, right now devedition _is_ slower
18:00bzBecause it has async steps on
18:00bzjryans: indeed
18:08SimonSapinbz: is &quot;That doesn&#39;t seem unreasonable&quot; about landing this now?
18:10SimonSapinemilio: r? and
18:10crowbotPR #18585: Use the current parser location for CSS error -
18:10crowbotPR #200: Include a SourceLocation in all error types -
18:18emilioSimonSapin: I&#39;ve skimmed for a bit over, and looks pretty ok to me. Is there anything in particular I should look up with more detail?
18:18crowbotPR #200: Include a SourceLocation in all error types -
18:21ajeffreystandups: Presented the ET headlines at the weekly meeting.
18:21standupsOk, submitted #51789 for
18:47SimonSapinemilio: sorry, forgot to reply earlier. Nothing in particular as long as youre happy with landing this. Just dont land it yes, Ill coordinate across repos tomorrow
19:02emilioSimonSapin: ok, the general approach makes sense to me. You can land it with r=me as long as it&#39;s green. Just had a few nits to comment over, really
19:04emilioSimonSapin: I&#39;d appreciate if you could review #18798. It&#39;s really in the easy-optimization land, and fixes the Youtube regression while we figure out how to make it better in bug 1403839
19:04crowbotPR #18798: style: Optimize custom properties cycle removal. -
19:04firebot NEW, stylo: custom-properties dependency cycle resolving algorithm is unreliable
19:38SimonSapinemilio: smallvec looks good, but does the early continue do much given that we have the same is_empty() check inside walk() ?
19:39emilioSimonSapin: it avoids inserting in the already_visited bits, which helped in the profiles I&#39;m looking at (where most of the time is spent now in hashmap / hashset insertions)
19:41SimonSapinemilio: ok.
19:41SimonSapinemilio: could the value param be made not optional, if we move the if let around the recursive call?
19:41emilioSimonSapin: sure, sounds good
19:44SimonSapinr+ with that
19:46emilioSimonSapin: thanks!
19:47emilioSimonSapin: (that also allows moving the `references.is_empty()` check inside the `walk` function, which is nice)
19:47emilioSimonSapin: just did that, want to take a quick look?
19:55bstriewho&#39;s the head honcho for managing rust integration with firefox?
19:56larsbergbstrie: I believe froydnj is the official &quot;peer&quot; for Rust in Firefox ( )
19:56emiliobstrie: I guess rillian / ted / froydnj? I know about some bits too, if I can help
19:57emiliobstrie: (#build may be a good place to ask around questions I suspect)
19:58bstrielarsberg: thanks!
20:05tedbstrie: yeah, froydnj or rillian or i can likely help you
21:11avadacatavra|sfstandups: went to ghc, catching up on all of the things
21:11standupsI don&#39;t trust you, avadacatavra|sf, are you identified with nickserv?
21:11* avadacatavra|sf argh
21:12avadacatavrastandups: went to ghc, catching up on all of the things
21:12standupsOk, submitted #51800 for
21:12avadacatavrastandups: behavioral interviewing tomorrow
21:12standupsOk, submitted #51801 for
21:13avadacatavrastandups: fixing pwm pr
21:13standupsOk, submitted #51802 for
21:13avadacatavrastandups: working on crypto workshops for mozfest
21:13standupsOk, submitted #51803 for
21:32dumbbellI&#39;m unable to build Firefox 56 with Rust components enabled on FreeBSD 11 on x86 (32-bit). The build fails inside the `style` crate because the build script segfaults:
21:32dumbbellFirst, am I in the right place to discuss this?
21:42gwdumbbell: This is probably a reasonable place to ask (or at least find out who to talk to), but I&#39;m not sure there&#39;s anyone online right now that would know the answers to that.
21:42gwbholley: emilio: ted: ^
21:43dumbbellgw: Thanks!
21:43bholleydumbbell: this sounds like
21:43firebotBug 1406952 NEW, Stylo build script SIGSEGV on Linux/x86
21:44bholleydumbbell: I have heard of people ooming in the build script
21:44emiliodumbbell: probably #build, too, but yeah, looks like that or
21:45firebotBug 1401093 NEW, stylo doesnt build on 32-bits
21:45dumbbellbholley: Great, thank you!
21:46SimonSapindumbbell: could OOM cause a segfault like this?
21:47dumbbellSimonSapin: I didn&#39;t see any evidence of a unusual memory consumption and the kernel would have reported a process being killed because of out-of-memory. I don&#39;t see anything in the logs
21:48SimonSapinI dont know if itd be enough to exhaust a 32-bit address space, but its possible that bindgen uses a lot of memory while parsing header files for most of Gecko
21:48emiliodumbbell: I just posted a patch in that you could try, if it happens to be OOM. If it&#39;s a crash on libclang itself it may also help, I suspect (if clang has some kind of race condition).
21:48dumbbellSimonSapin: I will double-check because the build happenss in a 32-bit container on top of a 64-bit host
21:48SimonSapinor it could be some other bug in bindgen, if its the same as bug 1406952
21:49dumbbellThank you all, I&#39;ll try the patch and report back
21:50emiliodumbbell: I&#39;ll probably be asleep by then, but feel free to comment on the bug
21:50dumbbellSure :)
21:50dumbbellHave a good night then!
22:19emiliodumbbell: I hadn&#39;t heard of before, but I think I know the underlying reason. If you happen to crash at the same point in the execution of the script, read below for a fix
22:19firebotBug 1406952 NEW, Stylo build script SIGSEGV on Linux/x86
22:23dumbbellemilio: Thanks, I&#39;ll try that as well after the current build
22:40xidornemilio: ping
22:41emilioxidorn: pong
22:41xidornemilio: re your comment in bug 1403839
22:41firebot NEW, stylo: custom-properties dependency cycle resolving algorithm is unreliable
22:41emilioxidorn: (-ish), about to leave
22:41xidornemilio: I&#39;m not sure I understand the algorithm you described
22:41xidornemilio: what does edge mean in your description?
22:42xidornemilio: is that the reference?
22:42xidornor variable?
22:42emilioxidorn: it&#39;s effectively all the references we visit from a given variable, plus that variable itself
22:42emilioxidorn: so right now, in substitute_all(), we go through each variable that has references, and walk the dependency graph effectively, substituting stuff
22:43xidornemilio: so, consider { --a: var(--b); --b: var(--a); --c: var(--a, 1); }, what would --c be in your algorithm?
22:43xidornand { --a: var(--b, 1); --b: var(--a); } what would --a be?
22:44emilioxidorn: I believe `--c` should be invalid, and `--a` too, since all of them are in a cycle, right?
22:45xidornemilio: in the first example, --c should be 1
22:45emilioxidorn: but we can implement both in an easy way, I believe
22:45xidornand in the second, --a should be invalid
22:45emilioxidorn: can you point me to the spec? I gave it a try this morning and the couple test failures I got back look a lot like that :)
22:46emilioxidorn: s/the spec/the right section of the spec/, though I can also look it up now (I&#39;m on my phone though)
22:46xidornemilio: those two are exactly the failures I saw when I tried to implement an &quot;easy&quot; algorithm for that
22:47xidornemilio: this is about the cycle resolving
22:47xidornemilio: basically everything in cycle should be invalid regardless whether then have fallback, and everything referring cycle is only invalid when there is no fallback (because themselves are not marked invalid by the cycle resolution)
22:48emilioxidorn: yup, got it. I think I know what&#39;s left in my patch now :D
22:48emilioxidorn: let me try to sketch it
22:49emilioxidorn: well, actually, let me grab my laptop
22:50xidornemilio: I guess your algorithm means you resolve variables when they need resolving, is it?
22:51emilioxidorn: right, pretty much
22:51xidornemilio: and you start the resolving from where it is requested?
22:51xidornso you can put everything it references into a set
22:51emilioxidorn: we go through all of the variables and resolve each one individually
22:51emilioxidorn: 1 sec
22:52xidorn(that sounds very similar to my first attempt, which hits the two failures mentioned above)
22:54emilioxidorn: yeah, the point is when do you remove from the map, I believe, and you cannot stop resolving once you find a cycle
22:54emilioxidorn: tl;dr, here&#39;s the commit (, but that has several problems right now.
22:55emilioxidorn: but I&#39;m moderately sure it can be made to work in this way
22:55xidornemilio: you need to clearly mark whether a variable is in loop or not
22:56xidornrather than collecting everything in a same set
22:56emilioxidorn: right, that&#39;s one of the problems
22:56emilioxidorn: maybe we don&#39;t need to collect them immediately
22:57xidorna naive DFS may have exponential complexity... but can be fast in general case where there&#39;s not cycle at all
22:58emilioxidorn: I believe we can do with two sets
22:58emilioxidorn: one of all the variables in a given graph starting from a single variable, and one for the ones that belong to t