Re: [webkit-dev] parallel painting
Hi, Thank all of you for your feedbacks! Let's continue this discussion in the bug report, I will provide more details there. Zoltan On Sat, Apr 3, 2010 at 9:32 PM, Zoltan Herczeg zherc...@inf.u-szeged.hu wrote: Hi, I am working on a parallel painting feature for WebKit (bug id: 36883). Basically it records the painting commands on the main thread, and replay them on a painting thread. The gain would be that the recording operation is cheap. Currently it is Qt specific, but I could make it more platform independent if other ports are interested. EFL port would be interested in this. Could you provide more details on the implementation? Is the painting thread a single thread, or is it being split to N cores? Did you consider the alternative that is isolate webkit layout in another thread as well? From our environment tests (embedded systems), re-layout process can take few seconds without any paint... that being done in the main thread hurts the whole experience as the event processing of menus, animations and others are blocked. In an ideal world WebKit would never block, it would just proxy input and output events to its thread (hard, error-prone... :-/), there it would layout, render and when ready release the main thread to use the painted contents (maybe tiles as the Qt port now enables). BR, -- Gustavo Sverzut Barbieri http://profusion.mobi embedded systems -- MSN: barbi...@gmail.com Skype: gsbarbieri Mobile: +55 (19) 9225-2202 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] A post-mordem of today's tree redness
On Mon, Apr 5, 2010 at 11:27 PM, Brent Fulgham bfulg...@gmail.com wrote: On Apr 5, 2010, at 9:58 PM, Adam Barth wrote: We had some trouble today keeping the tree green. In this email, I present a post-mordem analysis of what happened and what we can learn from these events. I've removed most of the names from this account because the purpose isn't to assign blame but to document what happened in the hopes that we can learn from it. Could rollout of patches be automated in some fashion, so that a previously-green tree becoming red could trigger a rollout of the last checkin? We have the tools to do this currently, but the prevailing wisdom is that we should have some human judgement involved in the process. We still have enough test flakiness that false positives could roll out perfect good patches. In other cases, it's clear how to fix the tree without rolling out. On the other hand, I've noticed that the varying speed of the various build bots makes it difficult to assess which patch might have triggered a break. It's not uncommon for some machines to be several patches behind others, and long test runs further exacerbate the problem. The sheriffbot does a pretty good job of narrowing down the regression window. Its algorithm is somewhat robust to flaky tests and other bits of noise. We continue to refine it based on experience. For example, even during today's complex overlapping failures, it correct computed the regression window to a set of five commits. The main failure mode we're seeing currently is that if a test fails 80% of the time, sheriffbot will generate false positives because it thinks that the test was fixed the one time it happens to be green. I have some ideas for how to handle that case, but we'll need to experiment some more. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Alignment issues in MIPS
Hello list, I'm cross-compiling WebKit (revision 40084) for a MIPS board using DirectFB backend. While executing in the target, libwebkit is causing SIGBUS errors due to misaligned double data. The errors raise when loading a misaligned double in the FPU using the ldc1 instruction (it is stated in the MIPS documentation that data loaded in the FPU must be 8-byte aligned to avoid an address error). I know that the kernel can be configured to manage misaligned accesses but i prefer not to mess with that by the moment. After some debugging, i found out that the offending variable is the attribute m_nextFireTime in the TimerBase class, which sometimes is not aligned to an 8-byte boundary. As a test, i tried compiling with -msoft-float (emulate FPU by software) and checked that the dissassembled code does not use the FPU but, in that case, webkit does not render anything. I also tried using the __attribute__ aligned directive without any success. Finally, i made an ugly patch to avoid the load from an unaligned address and it worked fine but i don't know if there are other places in the code whith the same problem. I have 2 questions: 1) Has anyone found similar alignment problems compiling webkit for MIPS? If so, how were they fixed? 2) Is there any standard way of forcing alignment of doubles to 8 bytes boundary through compiler options (i'm using gcc 4.3.2)? Regards, -- Alejandro Vazquez Fente ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] parallel painting
Parallel painting would only be useful if the graphics layer is incredibly slow. In most WebKit ports we do not see very much time painting, rather time is more often spent in layout, style resolution, or javascript execution/bindings. -eric On Sat, Apr 3, 2010 at 10:32 PM, Zoltan Herczeg zherc...@inf.u-szeged.hu wrote: Hi, I am working on a parallel painting feature for WebKit (bug id: 36883). Basically it records the painting commands on the main thread, and replay them on a painting thread. The gain would be that the recording operation is cheap. Currently it is Qt specific, but I could make it more platform independent if other ports are interested. Zoltan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Alignment issues in MIPS
On Tue, Apr 6, 2010 at 9:21 AM, Alex Vazquez alexvazquezfe...@gmail.com wrote: Hello list, I'm cross-compiling WebKit (revision 40084) for a MIPS board using DirectFB backend. While executing in the target, libwebkit is causing SIGBUS errors due to misaligned double data. The errors raise when loading a misaligned double in the FPU using the ldc1 instruction (it is stated in the MIPS documentation that data loaded in the FPU must be 8-byte aligned to avoid an address error). I know that the kernel can be configured to manage misaligned accesses but i prefer not to mess with that by the moment. I have SIGBUS problems cross-compiling on SH4 with Qt, similar to yours. Debug builds work fine, but release builds crash as soon as the first webkit-related call is made. I've not had time to investigate further, though I'll soon have to. I'm looking into your suggestion, and I see the class is not optimallly laid out (there is a function pointer which is presumably 4 bytes, followed by a double which is 8), though the compiler should correctly align it putting a 4 byte hole between the pointer and the double. At new time, the class should be allocated from a correctly aligned memory block, so any misalignments are probably due to copying the class around? After some debugging, i found out that the offending variable is the attribute m_nextFireTime in the TimerBase class, which sometimes is not aligned to an 8-byte boundary. As a test, i tried compiling with -msoft-float (emulate FPU by software) and checked that the dissassembled code does not use the FPU but, in that case, webkit does not render anything. I also tried using the __attribute__ aligned directive without any success. Well, that confirms it, the member variable was already aligned anyway. Finally, i made an ugly patch to avoid the load from an unaligned address and it worked fine but i don't know if there are other places in the code whith the same problem. I have 2 questions: 1) Has anyone found similar alignment problems compiling webkit for MIPS? If so, how were they fixed? 2) Is there any standard way of forcing alignment of doubles to 8 bytes boundary through compiler options (i'm using gcc 4.3.2)? -- Luciano Montanaro Anyone who is capable of getting themselves made President should on no account be allowed to do the job. -- Douglas Adams ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Windows mobile build
On Sun, Apr 4, 2010 at 10:45 AM, Patrick Roland Gansterer par...@paroga.com wrote: Hi, i had the same problem some months ago (https://lists.webkit.org/pipermail/webkit-dev/2010-January/011160.html), but didn't get any final reply. I had a look onto other (meta) buildsystems, but most of them lack WinCE (cross compile) support. I also tried gyp, but didn't get it working out of the box for WinCE. In my opinion qmake has the best support for building on WinCE at the moment. I also had the same idea like Kwang Yul. I think this (python) script should also generate all the other buildsystems (vcproj, xcode, GNUmake, qmake, ...). Then only this script must be changed when a file was added or removed. If I'm not wrong gyp was born to fulfil exactly this task? The best solution might be to add additional generators (e.g for the Qt-port) to gyp and then remove all other buildsystems from the tree? If somebody knows the long-term goal of the webkit buildsystem I'm very interested. If gyp is the preferred solution I might also spend some time with the WinCE part for it. The future of build systems in WebKit seems like a great topic for the meeting. It's already kind of ridiculous how much effort it requires to add one file. I'd hate for it to get that much more complex. - Patrick KwangYul, have you considered using gyp (and the files already there) rather than add another generating solution? Jason, have you considered using gyp to generate your solution? Disclaimer: I have no stake in gyp nor do I think it is necessarily better than anything else, but I do care about not modifying 7+ build files for any file additions/moves, etc. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] parallel painting
Date: Mon, 5 Apr 2010 17:54:35 -0800 From: barbi...@profusion.mobi To: zherc...@inf.u-szeged.hu CC: webkit-dev@lists.webkit.org Subject: Re: [webkit-dev] parallel painting On Sat, Apr 3, 2010 at 9:32 PM, Zoltan Herczeg wrote: Hi, I am working on a parallel painting feature for WebKit (bug id: 36883). Basically it records the painting commands on the main thread, and replay them on a painting thread. The gain would be that the recording operation is cheap. Currently it is Qt specific, but I could make it more platform independent if other ports are interested. EFL port would be interested in this. Could you provide more details on the implementation? Is the painting thread a single thread, or is it being split to N cores? Did you consider the alternative that is isolate webkit layout in another thread as well? From our environment tests (embedded systems), re-layout process can take few seconds without any paint... that being done in the main thread hurts the whole experience as the event processing of menus, animations and others are blocked. In an ideal world WebKit would never block, it would just proxy input and output What prevents this right now? That is, what puts work on an event thread instead of launching a worker thread? Generally of course event threads handle and record events- set flags, manage workers, maybe do simple things with existing data but no IO or unpredictable operations that may encounter contention( block). Certainly you don't want to use it to execute things that may require user intervention. I guess besides watching the disk light blink for VM while I'm trying to do something simple, I've personally found little more that is annoying than an unresponsive GUI thread. From the developer standpoint, you are stuck with options for responding to exceptional or pathological conditions with, say webpages or network conditions, if you can't count on a responsive GUI thread to process user input. I guess on limited resource devices threads are limited and launching new threads does create problems, often slowing things down overall when parallelism is the intent, but in this case you are only trying to preserve one thread's response time. Indeed, on phone's the first rule has been don't hang the UI thread since this allows user to report bugs and have a live menu system for recovery from wierd conditions. events to its thread (hard, error-prone... :-/), there it would layout, render and when ready release the main thread to use the painted contents (maybe tiles as the Qt port now enables). BR, -- Gustavo Sverzut Barbieri http://profusion.mobi embedded systems -- MSN: barbi...@gmail.com Skype: gsbarbieri Mobile: +55 (19) 9225-2202 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev _ Hotmail is redefining busy with tools for the New Busy. Get more from your inbox. http://www.windowslive.com/campaign/thenewbusy?ocid=PID28326::T:WLMTAGL:ON:WL:en-US:WM_HMP:042010_2 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] parallel painting
Of course, I did a lot of profiling before (using oprofile), and painting was slow on my platform. Zoltan Parallel painting would only be useful if the graphics layer is incredibly slow. In most WebKit ports we do not see very much time painting, rather time is more often spent in layout, style resolution, or javascript execution/bindings. -eric On Sat, Apr 3, 2010 at 10:32 PM, Zoltan Herczeg zherc...@inf.u-szeged.hu wrote: Hi, I am working on a parallel painting feature for WebKit (bug id: 36883). Basically it records the painting commands on the main thread, and replay them on a painting thread. The gain would be that the recording operation is cheap. Currently it is Qt specific, but I could make it more platform independent if other ports are interested. Zoltan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] parallel painting
On Tuesday 06 April 2010 13:42:30 Zoltan Herczeg wrote: Of course, I did a lot of profiling before (using oprofile), and painting was slow on my platform. Can you use oparchive to make the profile available? Also when you say painting? Do you mean function (inclusive) spend below QWebFrame::render? or really the work on QPainter and the RenderTree doing the painting? ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Alignment issues in MIPS
06.04.2010, в 00:21, Alex Vazquez написал(а): I'm cross-compiling WebKit (revision 40084) for a MIPS board using DirectFB backend. This is a very old revision. 1) Has anyone found similar alignment problems compiling webkit for MIPS? If so, how were they fixed? We have several related bugs, often with proposed solutions. Someone needs to rework the proposed solutions into a form that can be landed into the tree, see http://webkit.org/coding/contributing.html. Here are the bugs I could quickly find (I'm also listing Sparc and Alpha issues here, as they may have common root causes). https://bugs.webkit.org/show_bug.cgi?id=29415 [Qt] byte alignment issue on MIPS https://bugs.webkit.org/show_bug.cgi?id=19775 Alignment problems on Sparc https://bugs.webkit.org/show_bug.cgi?id=20990 FreeBSD Alpha, 3000 cast alignment warnings on build, unaligned access errors on run https://bugs.webkit.org/show_bug.cgi?id=19946 Possible misalignment in RenderArena when compiled for debug https://bugs.webkit.org/show_bug.cgi?id=17422 get the NULL stack base in MIPS https://bugs.webkit.org/show_bug.cgi?id=35461 Fail to build on sparc https://bugs.webkit.org/show_bug.cgi?id=35326 [patch] _atomic_word not always an int and WTF_USE_JSVALUE32 needs defining for 64 bit sparc linux https://bugs.webkit.org/show_bug.cgi?id=29407 [Qt] Webkit on SPARC Solaris have wrong endian - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] A post-mordem of today's tree redness
05.04.2010, в 21:58, Adam Barth написал(а): Leaving failures in the tree make it difficult to track down subsequent failures. Rolling out a change means more work for you, but less costs imposed on the rest of the project. While I agree with your analysis for the most part, there are costs associated with rolling out patches that you didn't mention. Some of these are: 1) Confusion about what is going on with the project. It becomes harder to know what's going on by reading webkit-changes - because you can't unsee a patch you saw landed, and because people often roll out patches with cryptic messages (roll out rX, because it breaks Tiger - how are you supposed to know that an important change you saw landed a few hours ago isn't there any more?) 2) Confusion also happens in Bugzilla - there are several styles for dealing with such issues (make a new bug for rollout, or just roll out and reopen). People often forget to document what they were doing to fix the build, so you end up with a resolved bug for something that has been rolled out, or a reopened bug without adequate explanations. Even when the original bug is correctly reopened, it can be hard to figure out its history, because commit queue clears out flags on patches. 3) Likelihood of more world rebuilds for developers and bots. A troublesome patch is more likely to touch common headers than a targeted build fix, so you get three world rebuilds instead of one. 4) It's harder to isolate regressions if these appear and disappear several times (aforementioned confusion doesn't help either). Screening bugs about regressions also becomes more error-prone. This arguments goes both ways though - it's even harder to isolate regressions if the platform in question had broken build at the time. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
05.04.2010, в 22:46, Maciej Stachowiak написал(а): The current implementation allows for (and operates on) positions such as [img, 0] - [img, 1] or [br,0] - [br, 1]. Is there a fundamental reason to keep such positions within the internal representation rather than normalize them to [parent-of-img, index-of-img(+1)] - round-tripping perhaps? Having fake positions like that is not good. I don't think there is a good reason for it. But the assumption got deeply embedded into the code, and it will take some doing to remove. One step in that direction would be to phase out all use of Position.deprecatedEditingOffset() and Position.node(). I think that one reason is performance - index-of-img can be costly. But I completely agree that we should try to get rid of these. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
On Apr 6, 2010, at 9:29 AM, Alexey Proskuryakov wrote: 05.04.2010, в 22:46, Maciej Stachowiak написал(а): The current implementation allows for (and operates on) positions such as [img, 0] - [img, 1] or [br,0] - [br, 1]. Is there a fundamental reason to keep such positions within the internal representation rather than normalize them to [parent-of-img, index-of-img(+1)] - round-tripping perhaps? Having fake positions like that is not good. I don't think there is a good reason for it. But the assumption got deeply embedded into the code, and it will take some doing to remove. One step in that direction would be to phase out all use of Position.deprecatedEditingOffset() and Position.node(). I think that one reason is performance - index-of-img can be costly. But I completely agree that we should try to get rid of these. Let me elaborate on what Maciej and Alexey said. If we have an img element, there are three valid DOM positions adjacent to it: A: [ parent-of-img, child-offset-of-img ]: before the image B: [ img, 0 ]: inside the image C: [ parent-of-img, child-offset-of-img + 1 ]: after the image WebKit’s HTML editing code also makes use of an invalid DOM position: D: [ img, 1 ]: inside the image, but used to represent the position after the image Using D is never a good idea because it’s not a valid DOM position, so we can’t use it to construct a DOM range object. Having positions like D in our code at all leads to complexity and confusion. Using B to represent the position before the image is also unconventional; the position is clearly “inside the image element”, whatever that means. Given that, it seems that editing-related code that starts with a position such as B should quickly convert it to either A or C as appropriate. But computing A or C given a pointer to an image element is costly because it involves walking the parent's child nodes. In a large, flat document the cost is proportional to the size of the document. The same goes if we start with A or C and want to find the image element just before or just after the position. There are other issues when the document is being changed with positions extant. If we add a child element to the img element’s parent before the img element, both A and C end up pointing to a different place in the document, but B and D will still point where they did before. So if we convert editing code that currently uses a position like B to instead use a position like A, it’s easy to change behavior without realizing it. Encapsulating some of this into a class is one of the goals of the Position object, but we haven’t made much progress on untangling the mess. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
See my comments below. Thanks. – Ken On Apr 5, 2010, at 10:30 PM, Roland Steiner wrote: One additional question on position classes: The current implementation allows for (and operates on) positions such as [img, 0] - [img, 1] or [br,0] - [br, 1]. Is there a fundamental reason to keep such positions within the internal representation rather than normalize them to [parent-of-img, index-of-img(+1)] - round-tripping perhaps? This is my fault. I made the initial error years ago to work with positions like [img, 0] - [img, 1] and [br,0] - [br, 1]. Though this is a mistake from the perspective of communicating positions outside of editing code, it is still very useful to know if a position is right before or after an image or br element. Some conveniences to ask such questions might be nice. If you wish to touch all the places in editing code where these degenerate positions are used, I have no reason to object. Cheers, - Roland On Fri, Apr 2, 2010 at 2:43 PM, Roland Steiner rolandstei...@google.com wrote: Hi all, As I am working on WebKit rich text editing these days, there are 2 issues that I would like to address. From a brief internal discussion both seem feasible and worthwhile, but since they involve changes to current code and behavior I wanted to ask the WebKit community in general, and the original authors of WebKit editing in particular, about your opinion: .) When a selection that starts in a table and ends outside it is deleted, the current code drags the adjacent outside content into the table. To me this is counter-intuitive (text can be dragged in, but not between cells, and not back outside), and it's also contrary to the behavior of other editors (FireFox, TextEdit, Word, etc.). The behavior is, however, enshrined in various layout tests, so I wonder if there was/is a reason to implement it this way. As this behavior also complicates fixing other bugs I wanted to see whether there would be much opposition to changing it (i.e., to content outside of a table staying outside on a delete operation). For my part, layout tests check behavior, they don't define it. They are meant to prevent regressions due to mistakes and unintended consequences, not to prevent genuine behavior improvements. .) The current Position classes are IMHO rather unfocused in their implementation, with lots of special cases and magical behavior, that still is often incorrect (e.g., with text that has padding, margins, or :before/:after content). For ease of further development they would therefore benefit from refactoring. The idea would be to change the classes into something along the lines of: DOMPosition: based on the current RangeBoundaryPoint, working on node + offset, interfacing with JavaScript EditingPosition (or TypeablePosition): based on the current PositionIterator for fast iteration, with most of the code of Position except for code that queries renderers VisiblePosition: change to work on renderers rather than nodes (moving such code from the current Position into this class). with explicit, but not implicit, conversion between them. Similarly for Ranges. In addition, a refactoring could add (or at least allow for) non-contiguous ranges and allow editing operations to work on arbitrary ranges/positions rather than just the (single) selection, which again currently is a pain point. In the long run I would envision to extend the code to allow multiple selections (such as for concurrent editing, or highlighting of find results, etc.), but that probably needs to be discussed separately. Yes, there is much magic in HTML editing. I always found it very difficult to make simple rules and adhere to them due to the complications of handling all the corner cases. So, it's hard for me to comment on your simply-stated proposals, since the implications of such design changes are vast. It's not clear that such a refactoring would make the code easier to change or maintain, and that seems to be a much more important goal than implementing any single feature (e.g. non-contiguous ranges). All that said, I am years removed from thinking about these problems on a regular basis. Mostly, I just wanted to own up to the position design mistake discussed at the top of the message. If only that were the greatest of my transgressions. It would be great if you could share your thoughts, Cheers, - Roland ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] A post-mordem of today's tree redness
On Tue, Apr 6, 2010 at 9:25 AM, Alexey Proskuryakov a...@webkit.org wrote: 05.04.2010, в 21:58, Adam Barth написал(а): Leaving failures in the tree make it difficult to track down subsequent failures. Rolling out a change means more work for you, but less costs imposed on the rest of the project. While I agree with your analysis for the most part, there are costs associated with rolling out patches that you didn't mention. Some of these are: 1) Confusion about what is going on with the project. It becomes harder to know what's going on by reading webkit-changes - because you can't unsee a patch you saw landed, and because people often roll out patches with cryptic messages (roll out rX, because it breaks Tiger - how are you supposed to know that an important change you saw landed a few hours ago isn't there any more?) I don't read webkit-changes, so I might not fully appreciate this use case, but the way I know when things are rolled out is because we reopen the bug and comment that patch was rolled out in a certain revision. If you like, we can put more information in the ChangeLogs created by sheriffbot (such as the title of the original bug). 2) Confusion also happens in Bugzilla - there are several styles for dealing with such issues (make a new bug for rollout, or just roll out and reopen). People often forget to document what they were doing to fix the build, so you end up with a resolved bug for something that has been rolled out, or a reopened bug without adequate explanations. Even when the original bug is correctly reopened, it can be hard to figure out its history, because commit queue clears out flags on patches. This problem is solvable with tooling. The process I recommend is as follows: 1) Open a new bug for the rollout patch and mark it blocking the main bug. This reduces noise on the main bug and provides a location to discuss the failures and resolve the situation, either by landing the rollout or not. (Creating a new bug is already automated.) 2) When landing a rollout, reopen the original bug and comment that the patch was rolled out and provide a link to the revision and bug. (Currently, this step is manual, but we can automate this too.) 3) Likelihood of more world rebuilds for developers and bots. A troublesome patch is more likely to touch common headers than a targeted build fix, so you get three world rebuilds instead of one. I don't see this as much of a concern. We can track statistics, but I bet the build time attributable to rollouts is less than 5% of all build time. 4) It's harder to isolate regressions if these appear and disappear several times (aforementioned confusion doesn't help either). Screening bugs about regressions also becomes more error-prone. This arguments goes both ways though - it's even harder to isolate regressions if the platform in question had broken build at the time. Concretely, supposed we hadn't cleaned up the Tiger bot to be green recently. I strongly suspect the regression caused by r57081 would have been lost in the thought process that the Tiger bot is always red. Even though the regression was real and affected every platform. Had we noticed the problem (say) a month later, we would have had a devil of a time tracking down the issue as evidenced by the effort required to fix the previous ancient Tiger-only failures. Even more concretely, the Windows bot have been red for thousands of revisions. Today someone (it's not important who) broke the break-blockquote-after-delete.html test on all the bots. He resolved the situation by updating the expected results to make the tree green again. However, he did not update the Windows expected results even though the failure diff is identical: http://build.webkit.org/results/Windows%20Release%20(Tests)/r57153%20(10992)/editing/inserting/break-blockquote-after-delete-pretty-diff.html That means when we finally get around to tracking down the failures some number of months ago, we'll be mystified by this failure even though we had the knowledge yesterday to fix the failure in a few minutes. I strongly suspect that if the Windows bots had started the day green and we had a culture of keeping the green, this individual would have made them green again and we wouldn't be accumulating a debt of mysterious failures that will drain our productivity in the future. I'm not saying the rollouts are always the best solution. For example, updating the expected results for break-blockquote-after-delete.html yesterday appears to have been the proper response. What I am saying is that we should keep the bots green, ideally by steadfastly refusing to regress tests. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] A post-mordem of today's tree redness
On 06.04.2010, at 10:06, Adam Barth wrote: I don't read webkit-changes, so I might not fully appreciate this use case, but the way I know when things are rolled out is because we reopen the bug and comment that patch was rolled out in a certain revision. If you like, we can put more information in the ChangeLogs created by sheriffbot (such as the title of the original bug). Yes, please do. I'm definitely not the only one reading webkit-changes and ChangeLogs. What I am saying is that we should keep the bots green, ideally by steadfastly refusing to regress tests. There has never been any disagreement about this, as far as I can tell. What seems to be an object for frequent discussions is whether disabling tests is an effective measure for making WebKit more secure and stable. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Let's get the Windows test bots green!
Hi all- The Windows test bots are red, and have been for a long time. Over the last day or so I've made sure we have bugs filed on all the failing tests, and I've tried to CC the appropriate people. The bugs are: https://bugs.webkit.org/show_bug.cgi?id=36217 Web Inspector: Inspector tests sometimes fail on Win Release https://bugs.webkit.org/show_bug.cgi?id=37089 fast/css/font-face-opentype.html fails on Windows https://bugs.webkit.org/show_bug.cgi?id=37091 fast/forms/text-control-intrinsic-widths.html fails on Windows https://bugs.webkit.org/show_bug.cgi?id=37096 http/tests/security/xss-DENIED-iframe-src-alias.html sometimes times out on Windows https://bugs.webkit.org/show_bug.cgi?id=37156 REGRESSION (r57113): editing/inserting/break-blockquote-after-delete.html fails on Windows https://bugs.webkit.org/show_bug.cgi?id=37157 REGRESSION (r57109): editing/pasteboard/drag-image-to-contenteditable-in-iframe.html fails on Windows If you have any ideas about how to fix these, please comment in the bug(s)! -Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] A post-mordem of today's tree redness
On 06.04.2010, at 10:06, Adam Barth wrote: 4) It's harder to isolate regressions if these appear and disappear several times (aforementioned confusion doesn't help either). Screening bugs about regressions also becomes more error-prone. This arguments goes both ways though - it's even harder to isolate regressions if the platform in question had broken build at the time. Concretely, supposed we hadn't cleaned up the Tiger bot to be green recently. I strongly suspect the regression caused by r57081 would have been lost in the thought process that the Tiger bot is always red. Even though the regression was real and affected every platform. Had we noticed the problem (say) a month later, we would have had a devil of a time tracking down the issue as evidenced by the effort required to fix the previous ancient Tiger-only failures. I was talking about regressions not covered by regression tests. When there is a bug about something that works in Safari 4, but not in ToT, you often need to find out when exactly it broke - and if there were several breakage points, it gets harder. Usually not by much, but sometimes this can lead you astray, and then it costs a lot. Again, this is not to say that we should never roll out patches, but it's another cost to consider. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Let's get the Windows test bots green!
Eric has some analysis scripts that know how to crawl back through the logs and find out when long-standing failures first occurred. We can ask him to run these scripts on the Windows bots when he wakes up. Adam On Tue, Apr 6, 2010 at 10:41 AM, Adam Roben aro...@apple.com wrote: Hi all- The Windows test bots are red, and have been for a long time. Over the last day or so I've made sure we have bugs filed on all the failing tests, and I've tried to CC the appropriate people. The bugs are: https://bugs.webkit.org/show_bug.cgi?id=36217 Web Inspector: Inspector tests sometimes fail on Win Release https://bugs.webkit.org/show_bug.cgi?id=37089 fast/css/font-face-opentype.html fails on Windows https://bugs.webkit.org/show_bug.cgi?id=37091 fast/forms/text-control-intrinsic-widths.html fails on Windows https://bugs.webkit.org/show_bug.cgi?id=37096 http/tests/security/xss-DENIED-iframe-src-alias.html sometimes times out on Windows https://bugs.webkit.org/show_bug.cgi?id=37156 REGRESSION (r57113): editing/inserting/break-blockquote-after-delete.html fails on Windows https://bugs.webkit.org/show_bug.cgi?id=37157 REGRESSION (r57109): editing/pasteboard/drag-image-to-contenteditable-in-iframe.html fails on Windows If you have any ideas about how to fix these, please comment in the bug(s)! -Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Let's get the Windows test bots green!
On Apr 6, 2010, at 1:50 PM, Adam Barth wrote: Eric has some analysis scripts that know how to crawl back through the logs and find out when long-standing failures first occurred. We can ask him to run these scripts on the Windows bots when he wakes up. Those scripts sound handy; can we get them checked into Subversion? Here are my guesses about when each failure started happening. (This information is in the bugs, too.): https://bugs.webkit.org/show_bug.cgi?id=36217 Web Inspector: Inspector tests sometimes fail on Win Release At least since r56095. https://bugs.webkit.org/show_bug.cgi?id=37089 fast/css/font-face-opentype.html fails on Windows This has likely been failing since it was first added in r54855. https://bugs.webkit.org/show_bug.cgi?id=37091 fast/forms/text-control-intrinsic-widths.html fails on Windows At least since r55975. https://bugs.webkit.org/show_bug.cgi?id=37096 http/tests/security/xss-DENIED-iframe-src-alias.html sometimes times out on Windows No idea. https://bugs.webkit.org/show_bug.cgi?id=37156 REGRESSION (r57113): editing/inserting/break-blockquote-after-delete.html fails on Windows https://bugs.webkit.org/show_bug.cgi?id=37157 REGRESSION (r57109): editing/pasteboard/drag-image-to-contenteditable-in-iframe.html fails on Windows The titles say it all. -Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Let's get the Windows test bots green!
On Tue, Apr 6, 2010 at 10:55 AM, Adam Roben aro...@apple.com wrote: On Apr 6, 2010, at 1:50 PM, Adam Barth wrote: Eric has some analysis scripts that know how to crawl back through the logs and find out when long-standing failures first occurred. We can ask him to run these scripts on the Windows bots when he wakes up. Those scripts sound handy; can we get them checked into Subversion? The code is under review in https://bugs.webkit.org/show_bug.cgi?id=36911. We would have landed it already, but I asked Eric to reorganize the code for better re-use in the future. The patch doesn't seem to apply to TOT anymore. I'm not sure how hard the merge would be. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] A post-mordem of today's tree redness
On Mon, Apr 5, 2010 at 11:35 PM, Adam Barth aba...@webkit.org wrote: On Mon, Apr 5, 2010 at 11:27 PM, Brent Fulgham bfulg...@gmail.com wrote: On Apr 5, 2010, at 9:58 PM, Adam Barth wrote: We had some trouble today keeping the tree green. In this email, I present a post-mordem analysis of what happened and what we can learn from these events. I've removed most of the names from this account because the purpose isn't to assign blame but to document what happened in the hopes that we can learn from it. Could rollout of patches be automated in some fashion, so that a previously-green tree becoming red could trigger a rollout of the last checkin? We have the tools to do this currently, but the prevailing wisdom is that we should have some human judgement involved in the process. We still have enough test flakiness that false positives could roll out perfect good patches. In other cases, it's clear how to fix the tree without rolling out. On the other hand, I've noticed that the varying speed of the various build bots makes it difficult to assess which patch might have triggered a break. It's not uncommon for some machines to be several patches behind others, and long test runs further exacerbate the problem. Another example of tree redness that I caused a couple weeks ago: 1. Commit a change that I thought would be green on all platforms. 2. Turned out to regress some tests on the Chromium Windows bot. 3. Committed a fix for Chromium Windows, which also fixed the WebKit Windows bots. The WebKit Windows bots had gotten hours behind, so the original commit had still not run the tests on the bots. Hours later, the original commit ran and the bot turned red, even though the fix was already committed. Hours after that, the followup commit ran and the tests were fixed. Anyways, my point is that it's really difficult to look at the state of the tree and say definitively that a patch should be rolled out. Also, I'd like to add a recommendation to improve keeping the tree green. Make the bots cycle faster. It makes it much easier to see what commit caused a regression and it makes it easier to tell that a fix actually greened the tree. Not sure what this involves, but some ideas: 1. Get faster/more machines. 2. Run tests in parallel. new-run-webkit-tests does this if we can get it to be a suitable replacement for run-webkit-tests. How much benefit we get from parallelizing depends on the number of cores on the machine, which gets back to recommendation 1. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] A post-mordem of today's tree redness
On Apr 6, 2010, at 10:35 AM, Alexey Proskuryakov wrote: On 06.04.2010, at 10:06, Adam Barth wrote: I don't read webkit-changes, so I might not fully appreciate this use case, but the way I know when things are rolled out is because we reopen the bug and comment that patch was rolled out in a certain revision. If you like, we can put more information in the ChangeLogs created by sheriffbot (such as the title of the original bug). Yes, please do. I'm definitely not the only one reading webkit-changes and ChangeLogs. I agree that more informative revert ChangeLog entries would be useful. My preference would be to include the following: - revision that was reverted - bugzilla bug URL from the ChangeLog of the reverted commit (in a distinct format from how we'd provide it if it was the bug for the rollout change itself) - summary text from the original revert Something like: - Reverted rN (Rotate head 90 degrees clockwise) http://bugs.webkit.org/rollout-bug-num Reverted bug was: http://bugs.webkit.org/bug-that-reverted What I am saying is that we should keep the bots green, ideally by steadfastly refusing to regress tests. There has never been any disagreement about this, as far as I can tell. What seems to be an object for frequent discussions is whether disabling tests is an effective measure for making WebKit more secure and stable. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] A post-mordem of today's tree redness
On Tue, Apr 6, 2010 at 11:43 AM, Maciej Stachowiak m...@apple.com wrote: On Apr 6, 2010, at 10:35 AM, Alexey Proskuryakov wrote: On 06.04.2010, at 10:06, Adam Barth wrote: I don't read webkit-changes, so I might not fully appreciate this use case, but the way I know when things are rolled out is because we reopen the bug and comment that patch was rolled out in a certain revision. If you like, we can put more information in the ChangeLogs created by sheriffbot (such as the title of the original bug). Yes, please do. I'm definitely not the only one reading webkit-changes and ChangeLogs. I agree that more informative revert ChangeLog entries would be useful. My preference would be to include the following: - revision that was reverted - bugzilla bug URL from the ChangeLog of the reverted commit (in a distinct format from how we'd provide it if it was the bug for the rollout change itself) - summary text from the original revert Something like: - Reverted rN (Rotate head 90 degrees clockwise) http://bugs.webkit.org/rollout-bug-num Reverted bug was: http://bugs.webkit.org/bug-that-reverted Okiedokes. I'll try to do that today. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] A post-mordem of today's tree redness
On Apr 6, 2010, at 11:57 AM, Adam Barth wrote: On Tue, Apr 6, 2010 at 11:43 AM, Maciej Stachowiak m...@apple.com wrote: On Apr 6, 2010, at 10:35 AM, Alexey Proskuryakov wrote: On 06.04.2010, at 10:06, Adam Barth wrote: I don't read webkit-changes, so I might not fully appreciate this use case, but the way I know when things are rolled out is because we reopen the bug and comment that patch was rolled out in a certain revision. If you like, we can put more information in the ChangeLogs created by sheriffbot (such as the title of the original bug). Yes, please do. I'm definitely not the only one reading webkit-changes and ChangeLogs. I agree that more informative revert ChangeLog entries would be useful. My preference would be to include the following: - revision that was reverted - bugzilla bug URL from the ChangeLog of the reverted commit (in a distinct format from how we'd provide it if it was the bug for the rollout change itself) - summary text from the original revert Something like: - Reverted rN (Rotate head 90 degrees clockwise) http://bugs.webkit.org/rollout-bug-num Reverted bug was: http://bugs.webkit.org/bug-that-reverted Okiedokes. I'll try to do that today. By the way, just to explain why I ask for this: I often search ChangeLogs for the history of an issue, and searching for the bug number or obvious related text will not generally find entries where a change was rolled out. I would have to already know that the change was rolled out at some point, find the revision number of the original change, and then search for ChangeLog entries mentioning that revision number. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev