Meeting 2013 12 09

Servo Meeting 2013-12-09

Agenda

• Introducing Isabelle Carter (intern, working on position:fixed)
• Workweek on 1/20 (https://etherpad.mozilla.org/Servo-workweek-2014-01 )
  • agenda ideas
  • who
• Rust Arm port. Really need Android make check not to break Android
• Rust pkg manager. Removal plan
• Servo Rust version update
• Android port pr. requirements from mozilla?
• Testing strategy. More conservative master branch?
• Removing @ from flowtree.
• review queue

Attending

azita, pcwalton, brson, dherman, jack, larsberg, jdm, kmc

Isabelle Carter introduction

• jack: Isabelle carter will be adding position: fixed. She's here from the GNOME Outreach Program for Women.
• larsberg: She will be here until March 10th

Workweek

• jack: Simon and jdm will be in SF on January 20th. Since most of the Servo team is already there, we will have a workweek then. A couple of you from Samsung are welcome to come, particularly to work on CSS stuff with us and Simon, if you are interested.
• ryan: We will see what we can do.
• jack: We have an etherpad for these ideas (above) with the wrong year (because of lars). Since Simon is the catalyst, we were thinking of doing CSS-related work. Generated content and tables are things we could get started on. It might also be nice to look at the HTML parser and maybe string interning. Those are all related to things that he works on and is active on. Anything else? Patrick on incr. reflow?
• kmc: Layerd display lists since we need them for Acid2. Also intersects with position: fixed, etc.
• pcwalton: Yes, we need layers in general for many features.
• jack: We are also going to invite ibnc, maybe daniel glazman, ms2ger can't make it, possibly jgraham. Is there anyone else we were thinking of inviting? We will discuss it next week as well with Simon. We will keep the workweek page updated, as we did for the one in Korea.

Rust ARM port

• ryanc: We really need the make check system so that future changes don't break our ARM port.
• pcwalton: brson? I think we need it for rust
• brson: We are adding android automation on rust. I spent several days working on it on EC2, but it did not work. I will dedicate hardware to it here and will be working on it again starting tomorrow. Should have it sometime before the new year.
• ryanc: We need to do our internal demo to executives frequently, like every quarter. Last demo, we had to redo all the android port for the demo, so we couldn't concentrate on servo and other stuff. In the future, we do not want to redo the things we did over and over. We'd like the system not to break our Android port. Otherwise, we are wasting our time redoing all of the things. Can we really ask to have Android building checking built in?
• brson: This is also servo on android.
• pcwalton: WE need it working for Rust first, right? I think we need it both for Rust and Servo. Jack, what's the servo status?
• jack: It's not clear how we will run tests on Android. First, we should get the android build working again. It's probably blocked on both my build stuff and a rust upgrade to at least compile as part of the build. That we can do again. BUild libservo.dylib so we at least don't break the code. Someone will need to make a harness that runs the android code. I am hoping that Samsung can work on that.
• brson: We have one on Rust that works and might be able to copy it to servo.
• jack: Maybe the null compositor and software rendering. If it's not going to work on EC2, then it will take more work for us.
• brson: It'll just take more work on EC2 and I was going to punt.
• jack: I can look at it as well. Maybe share some resources?
• brson: Possibly, I don't know how the VPC works. Your is set up in that, unlike ours.
• jack: I need to talk to bhearsum about how to set up a mozharness locally so I can test expanding our build changes more easily. The plan is to fix our build, get android building again (immediately, by end of year). The automated tests for android may take longer. Samsung should not have to be redoing work.

Rustpkg manager

• jack: It can't build all of the packages currently. And I don't think it ever got support for cross-compiling. I have a PR open in Rust (https://github.com/mozilla/rust/pull/10593 ). It makes the crate hashes stable, so that external tools (like cmake or make or tup) can make more reliable builds. We will set it up to make our builds more parallel and cleaner for cross-compiling to android. That PR should hopefully get updated tonight and reviewed/landed tomorrow. Once it does, I'll work on the Rust upgrade and build system.
• brson: Which build system?
• jack: cmake is the plan for now.
• pcwalton: Why that over tup?
• jack: A couple are already familiar with cmake. Cmake includes configuring. And it's a known-working thing. We'd miss tup's auto-parallel build issue detection. Not a huge problem because cmake, by default, will use the new rust dependency magic I'm adding and it already does that for C.
• lars: Tup also hard to get running on Windows and seemed to have some bugs when we were trying to use it.
• jack: tup and cmake were about the same speed - way, way faster than our current system. Huge improvement no matter which we use. We will remove rustpkg, the makefiles, and configure scripts entirely. The individual submodules will still be buildable with rustpkg (there's nothing special needed for that).

Rust version update

• ryanc: Related to our android port
• jack: To do the build work, I need this Rust PR, so we'll move to the current master once this patch lands. The biggest blockers are linked task failure need to be replaced, all uses of do need to be removed, all stack closures change signatures, owned closures -> procds
• kmc: I'm working on linked tasked failure and have a design that is pretty much what we want. Instead of task::try, there's a port you can get that tells you failure and is much cleaner.
• jack: The other conversions I've done so far have not been too bad, mostly mechanical. Hopefully it will be quick.
• kmc: That will also unblock intrusive flow list work which will improve layout performance significantly.

Requirements from Mozilla for android port

• jack: for rust or servo?
• ryanc: Almost ready for another PR for our android port. Do you have any requirements?
• jack: Nothing special. One thing is that we're not sure how to build a test harness to test android. That can come later, though, we should not block this.
• pcwalton: But the sooner we get a test harness, the less likely we are to break the Android port.
• jack: Hopefully the Android changes weren't terrible - we tried to be careful.

Demo this month

• ryanc: Even master was not passing acid1 and breaking some test cases, which hugely affected our demo preparation. If you have some test code, we could use a stable branch.
• jack: Yes, we need an acid1 reference in the reftest. And need to get ref tests running as part of make check because it was not running on one of the platforms. We should be able to do that. Other things? Oh, jdm has a bot that lets us know if the PR modifies unsafe code or layout code without reftests.
• pcwalton: Hard to reftest acid1, but maybe a content test.
• jack: I was just going to delete all the text from the boxes, create a PNG of the boxes, etc.
• pcwalton: I'll do that. I noticed that regression and backed it out as soon as I noticed it, but it had been for a day or two.
• jack: jdm has been pushing on us to add content tests for linux, and we should add ref tests at teh same time
• kmc: Anything known to be broken with content tests on linux?
• jack: Race condition in std:;process. It was crashing before wait_for_pid, so we need to see if that still happens, if it's a rust bug or our bug, etc. It never worked, but the failure started showing up when we started checking the return code. Also, servo crashing at shutdown was preventing us from being able to do it.
• kmc: I think I turned on checking the return code for content tests.
• jack: Should do it for reftests, too.
• kmc: Need not just a null compositor, but something more for reftests.
• jack: I volunteer lars!

Remove @ from flow tree

• ryanc: Working on that. Removed all of them from the flow tree. So, one thing I noticed is that the current version when you build a display list, it has a managed pointer to the boxes in the flow tree. When I was converting managed boxes to owned, I removed the connection between the display list and flow tree. This is temporary. Design decision: When we build the display list, do we really need a pointer to the box in the flow tree, because we don't use it, it just needs the coordinates.
• pcwalton: I'm working on removing it right now. I do not think we need the connection at all. DisplayList does not need a pointer to the box. Hopefully today or tomorrow I will have a PR removing it.
• jack: Need index?
• pcwalton: No, just a pointer to the node it came from. Because from the display list, you need to go back to the node for hit testing or getBoundingClientRect, both of which require nodes, not nodes. So you don't need pointers to a flow, box, etc.
• jack: how will this interact with GC? Now we have two new threads that have pointers to Node...
• pcwalton: Ick. That's a problem. Not a problem if we didn't keep display lists around. We could rebuild them every time.
• jack: We would send the coordinates to script, script asks layout to rebuild the display list, and find the node from that? It could pretend to build the display list, though, right?
• pcwalton: No, every display list item contains the address of a node inside it. But, the GC can move the node and invalidate the pointer. In practice it won't happen because we ensure that you rebuild the display list so that it never contains stale pointers (invalidation ensures no display items point to dead nodes), but it's scary that we rely on invalidation.
• jack: I don't get how this works with the GC. JS GC runs while layout is asleep, and...
• pcwalton: Yes, that can happen, but not while layout is running. So, if we rebuild the displaylist every time, then we know there will be no stale pointers to nodes in the display list.
• jack: Is the current JS GC moving?
• pcwalton: No, but we shouldn't make decisions that prevent it.
• jack: Can we enable moving GC in the current spidermonkey?
• pcwalton: No. We need ggc (generational garbage collection).
• jack: Would be nice to tests. This is scary.
• kmc: We'll have COW DOM and then JS GC can run during layout, right?
• pcwalton: Sure. Potential workaround: it's safe to compare node pointers for equality (getboundingclientrect & hit testing) against the node passed in. Even if it's stale, the worst case is that we return the wrong value.
• kmc: What about reuse problems with the pointer?
• pcwalton: Still, the worst case is getting the wrong response. Not a memory safety/security problem. There is a problem with hit testing. With it, there's no way around rebuilding the display list. But that's usually a mouse click, just a couple of milliseconds. As long as it only takes a couple of milliseconds, we can just rebuild the DisplayList for hit testing and then we don't have to worry.
• kmc: Idea is that on mouse click, we already have a flow tree, so just rebuild the display list.
• pcwalton: Yes. And mouse clicks don't happen more than once per frame, so we should be fine as long as we don't take > 16ms to handle (ed note: to stay at 60fps). With that, we can go back to storing AbstractNode in the DisplayList.
• jack: Question about the moving collector. What happens when it moves it? Will it use the tracing info to update them all?
• pcwalton: Anything AbstractNode is going to be dangerous. We will not be able to copy AbstractNode because of the moving collector. The more we write, the more trouble we will be in.
• jack: How does that affect your new design, jdm?
• pcwalton: AbstractNode<T> is copyable, but that's incompatible with exact rooting. It needs to be Rooted<JSManaged<T>>. To create another, you'd need to do .root() or something. To enforce that in Rust, you have to make it noncopyable. impl Drop for JSManaged.
• jack: Or the stupid zero-length field?
• pcwalton: Either.
• jack: Seems goofy to implement Drop just to get that behavior.
• pcwalton: Will probably have an attribute.
• jack: Or niko wants everything non-copyable.
• pcwalton: Attribute is the most sensible thing these days. The important thing is that we have to start moving towards AbstractNode being non-copyable. Everything in future-design should be compatible with moving GC.
• jack: Somebody should try making it non-copyable and see what breaks...
• pcwalton: Probably in the DOM bindings generatore?
• jdm: We don't use AbstractNode there.
• pcwalton: We should do that check sooner rather than later. Don't want to get too far down the road (like Blink) and be stuck with a conservative global GC.
• jack: This makes me think we need to reprioritize the SpiderMonkey work.
• pcwalton: We can't be fully safe with it in the Rust of today, but we know what we can do.
• jack: Because of rooting?
• pcwalton: Yes. But we have to fix AbstractNode first.
• jack: And some way to compile & inline C++.
• pcwalton: acrichton has been doing LTO in Rust that should dovetail nicely with it.
• jack: Short of the matter is, with new Flow tree changes, removing @ should be easier.
• pcwalton: I will get that in ASAP. Then we can merge the parallel layout stuff. I'd kinda like to wait on the Rust upgrade because it contains the Deque (Chase-Lev), so we don't have to copy/past it. And we can remove utils/slot.rs because it's been upstreamed.
• jack: RefCell, Cell, Slot, etc. need a blog post.
• pcwalton: They're the replacement for @mut
• jack: Slot vs. Cell? Why RefCell instead of mut? These are not obvious/clear.
• pcwalton: It's still an experiment.
• jack: Need to get the info out there.
• pcwalton: Cell is being removed/replaced. I'll blog about it all. Or maybe Niko will.

Review queue

• jack: Get on those PRs!
• jdm: Rotted ones from pcwalton
• jack: I did some this morning, will do more tomorrow. Some want input from pcwalton, simonsapin, jdm...
• pcwalton: I'll make a pass.

FontContext

• jack: PR I approved that had @mut FontContext in the LayoutContext changed to a method, so now it takes &mut self. So @mut FontContext was not parallelizable, but &mut self isn't either...
• pcwalton: Fine if you use MutexArc. Not necessarily parallelizable, but safe.
• jack: Maybe each thread will need a FontContext.
• pcwalton: Fonts are a bit of a hack, but not racy now, but we may have lots of contention on them. That MutexArc might be contented.
• jack: Should be totally read-only. Maybe a background process to add new fonts w/ messages, etc, and all the font structs can be immutable.