Re: [webkit-dev] malloc(0)

[Darin Adler]

On Jun 12, 2012, at 3:03 PM, Myles C. Maxfield  wrote:

> I noticed that this caused a problem in one particular place 
> (WTF::StringImpl::getData16SlowCase()) where the code allocates (constant * 
> length) bytes for an (empty) string, and provides an accessor that exposes 
> this pointer. This pointer was being passed to ICU, which didn't perform the 
> requested function because it looked like one of the arguments was invalid, 
> even though it was just empty.

I don’t think we should generalize from this one case to a requirement that 
malloc(0) return a pointer.

In the case you describe, it’s the ICU behavior is what seems strange to me. 
Almost all other libraries would be happy to take a null pointer and a length 
to represent a string of zero length. I suggest we put workaround close to the 
site of that strangeness. I would not object to having that workaround in place 
even though most platforms won’t run into the problem due to malloc behavior.

I think that Eric’s interest was in greater efficiency by changing our designs 
to avoid doing malloc(0). I have no strong objection to that either, but it 
seems worthwhile only if the cost is really significant.

-- Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] are fuzzer tests appropriate layout tests?

[Ojan Vafai]

See https://bugs.webkit.org/show_bug.cgi?id=87772.

It's great to use a fuzzer in order to find cases where we're broken and
then make reduced layout tests from those. The viewspec-parser tests are
themselves just a fuzzer though. Granted, they are deterministic by
avoiding using an actual random function, but I don't think throwing
randomly generated bits at a parser is appropriate for layout testing. If
nothing else it's very slow.

These tests regularly timeout on the Chromium debug bots and occasionally
timeout on the Apple Lion bots. Even on the bots where they don't timeout,
they're slow. I don't it makes sense to spend 1+ minutes running these 5
tests when more targeted reductions could get the same effective coverage
much faster.

Am I wrong? If not, does anyone object to moving these tests over to
ManualTests or just deleting them entirely?

Ojan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Adding css3-conditional's @supports rule

[Pablo Flouret]

Hi there,

I posted a patch [1] a while ago to add support for the @supports rule [2].

I wasn't sure if it was a big enough feature to warrant mailing  
webkit-dev, but since the bug hasn't moved much maybe it is.


There's a lot of anecdotal interest from devrel folk and developers, and  
from Mozilla as well (they're the editors of the spec) [3]


The patch i posted is not webkit-prefixed since i think it would  
half-defeat the feature's purpose, but i can go that way if it's deemed  
important.


What do you guys think?

[1] https://bugs.webkit.org/show_bug.cgi?id=86146
[2] http://www.w3.org/TR/css3-conditional/#at-supports
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=649740

--
pablo flouret
motorola | webkit / browser team
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] malloc(0)

[Adam Barth]

There was some discussion about how to handle malloc(0) a year or so
ago.  I don't remember if it was on webkit-dev, but you might want to
check the archives.  Eric Seidel might remember what conclusions (if
any) we came to.

Adam


On Tue, Jun 12, 2012 at 3:03 PM, Myles C. Maxfield
 wrote:
> Hello,
> I'm compiling WebKit with a malloc() implementation that returns NULL
> for malloc(0). According to C99, this is valid: "If the size of the
> space requested is zero, the behavior is implementation- defined:
> either a null pointer is returned, or the behavior is as if the size
> were some nonzero value, except that the returned pointer shall not be
> used to access an object."
>
> I noticed that this caused a problem in one particular place
> (WTF::StringImpl::getData16SlowCase()) where the code allocates
> (constant * length) bytes for an (empty) string, and provides an
> accessor that exposes this pointer. This pointer was being passed to
> ICU, which didn't perform the requested function because it looked
> like one of the arguments was invalid, even though it was just empty.
>
> I have worked around this one particular occurrence in my local
> version of WebKit fork, but I'm wondering how often this pattern
> occurs. Is my fix worth upstreaming?  Is it worth trying to find,
> "fix," and upstream every occurrence of this pattern? Or is this
> particular behavior of malloc() an unstated requirement of building
> WebKit? If the latter is true, perhaps it's worth explicitly stating
> this somewhere? What is the opinion of the community?
>
> Thanks,
> Myles
> ___
> 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

[webkit-dev] malloc(0)

[Myles C. Maxfield]

Hello,
I'm compiling WebKit with a malloc() implementation that returns NULL
for malloc(0). According to C99, this is valid: "If the size of the
space requested is zero, the behavior is implementation- defined:
either a null pointer is returned, or the behavior is as if the size
were some nonzero value, except that the returned pointer shall not be
used to access an object."

I noticed that this caused a problem in one particular place
(WTF::StringImpl::getData16SlowCase()) where the code allocates
(constant * length) bytes for an (empty) string, and provides an
accessor that exposes this pointer. This pointer was being passed to
ICU, which didn't perform the requested function because it looked
like one of the arguments was invalid, even though it was just empty.

I have worked around this one particular occurrence in my local
version of WebKit fork, but I'm wondering how often this pattern
occurs. Is my fix worth upstreaming?  Is it worth trying to find,
"fix," and upstream every occurrence of this pattern? Or is this
particular behavior of malloc() an unstated requirement of building
WebKit? If the latter is true, perhaps it's worth explicitly stating
this somewhere? What is the opinion of the community?

Thanks,
Myles
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Can we distinguish imported tests in LayoutTests/css3 ?

[Darin Adler]

On Jun 12, 2012, at 1:02 PM, Ryosuke Niwa wrote:

> Because then we'll be placing tests for one component in multiple directories.

Makes sense, but fleshing this out for all the tests could be tricky.

Only a certain percentage of our tests will fall cleanly into a component such 
as CSS or DOM.

-- Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Can we distinguish imported tests in LayoutTests/css3 ?

[Ryosuke Niwa]

On Tue, Jun 12, 2012 at 10:54 AM, Darin Adler  wrote:

> On Jun 12, 2012, at 10:17 AM, Ryosuke Niwa wrote:
>  > The last time we had this discussion, the consensus appeared to be
> renaming it to RegressionTests.
>
> I still like that name.
>
> > Are you suggesting that we have LayoutTests/W3C?
>
> Yes. Something like RegressionTests/ImportedTests/W3C.
>

I don't like that. Because then we'll be placing tests for one component in
multiple directories. e.g. if I'm modifying CSS parser and wanted to check
if my WIP is sane or not, I want to be able to run all tests for the CSS
component (of course, I'll need to run the entire layout test before
submitting the final patch).

If we added a top-level imported tests directory, then potentially I would
have to run a command like:
run-webkit-tests CSS ImportedTests/W3C/CSS* ImportedTests/Mozilla/CSS

I'd much prefer a solution where organize imported tests under each
component so that we'll have structures like:
RegressionTests

   - CSS
  - W3C
 - CSS1
 - CSS2.1
 - CSS3
  - Mozilla
 - Bidi
  - DOM
  - W3C
 - DOM-Level-1
 - DOM-Level-2

So that I can all tests for the CSS component by simply:
run-webkit-tests CSS

> we have at least 15 test running scripts
>
> Too many!
>

Yeah, I would like to reduce that number if possible. For example,
different kinds of API test runners can probably be merged into single
run-webkit-api-tests that takes various flags like --gtk and --qt much like
run-webkit-tests.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] removing js-test-post.js WAS: Can we distinguish imported tests in LayoutTests/css3 ?

[Alexey Proskuryakov]

12.06.2012, в 10:51, Alexey Proskuryakov написал(а):

>> I'm sympathetic to this argument. In this case, the only way it makes the 
>> tests more fragile is that it makes them depend on us firing the onload 
>> event. I feel like so many things would break if you break onload, that it's 
>> not really making the tests more fragile in practice. 
> 
> There are always edge cases that are complicated. No longer than yesterday, I 
> was helping someone with a test that mysteriously didn't print "Test 
> Complete" in ToT WebKit, while that worked in Firefox. Unfortunately, I 
> didn't capture the test back then, and cannot reproduce the problem right now 
> (we ended up using a whole different approach in that test).

Got it, filed .

This is probably a regression from earlier changes meant to "simplify" 
boilerplate.

- WBR, Alexey Proskuryakov

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] build.webkit.org down for upgrade

[Lucas Forschler]

Hi Ossy,

Can you send me the bugzilla link for the ForceScheduler work?

Thanks,
Lucas

On May 31, 2012, at 9:22 AM, Lucas Forschler wrote:

> HI Ossy,
> 
> I did notice the display order change as well.  I think I am going to open a 
> bug to rename all of the Apple bots to prefix them with 'Apple'.  We (at 
> Apple) would like to get our bots into a more conforming naming convention.  
> I realize that won't solve the problem with having a specific order for your 
> bots, but it will at least get everything grouped together.
> 
> I'm slightly opposed to rolling out the natural sorting as you suggest below… 
> I think that will be short-lived and when buildbot 0.9.x is available, we may 
> not have this option.  I think we should be forward thinking here and try not 
> to work around this.  I can be convinced to make the change if there is 
> additional support for rolling out the natural sorting.
> 
> Thanks for looking into the ForceScheduler.  Hopefully we can get that up and 
> running quickly.
> 
> So far it appears we don't have any bots stuck in trigger mode.  Please let 
> me know if you see any.  I am hoping that issue is now solved for good.
> 
> Thanks,
> Lucas
> 
> 
> 
> 
> On May 31, 2012, at 3:32 AM, Osztrogonac Csaba wrote:
> 
>> Lucas Forschler írta:
>>> to 0.8.6p1
>>> Will be back online when complete.
>>> Lucas
>> 
>> Hi All,
>> 
>> Unfortunately there are too annoying bugs introduced
>> with upgrading build master to 0.8.6p1 :
>> 
>> Problem 1
>> --
>> Builders are alphabetically ordered on http://build.webkit.org/waterfall
>> instead of the given order in our config.json.
>> 
>> http://buildbot.net/buildbot/docs/0.8.6p1/release-notes.html
>> "Builders are no longer displayed in the order they were configured. This 
>> was never
>> intended behavior, and will become impossible in the distributed 
>> architecture planned
>> for Buildbot-0.9.x. As of 0.8.6p1, builders are sorted naturally: lexically, 
>> but with
>> numeric segments sorted numerically."
>> 
>> I think the new alphabetical order is so confusing. For example Apple's Lion 
>> bots
>> are between GTK and Qt bots, but SnowLeopard bots are after the Qt bots. I 
>> know
>> that we can see category=AppleMac, category=Qt, ... , but the order of the 
>> bots
>> in a given category is important for us too.  I don't think that an artifical
>> renaming would be a good solution to get the order what we want. (Because we
>> would have to rename bots in all scripts, we would have loose the history, 
>> ...)
>> 
>> What about if we revert the buildbot-0.8.6p1 change in buildmaster caused 
>> this bug/feature?
>> Here is a simple patch to do it:
>> 
>> diff --git a/buildbot-0.8.6p1/buildbot/status/master.py 
>> b/buildbot-0.8.6p1/buildbot/status/master.py
>> index e803133..e60ab06 100644
>> --- a/buildbot-0.8.6p1/buildbot/status/master.py
>> +++ b/buildbot-0.8.6p1/buildbot/status/master.py
>> @@ -200,7 +200,7 @@ class Status(config.ReconfigurableServiceMixin, 
>> service.MultiService):
>> 
>>def getBuilderNames(self, categories=None):
>>if categories == None:
>> -return util.naturalSort(self.botmaster.builderNames) # don't 
>> let them break it
>> +return self.botmaster.builderNames # don't let them break it
>> 
>>l = []
>># respect addition order
>> @@ -208,7 +208,7 @@ class Status(config.ReconfigurableServiceMixin, 
>> service.MultiService):
>>bldr = self.botmaster.builders[name]
>>if bldr.config.category in categories:
>>l.append(name)
>> -return util.naturalSort(l)
>> +return l
>> 
>>def getBuilder(self, name):
>>"""
>> 
>> Problem 2
>> --
>> There isn't force build possibility anymore.
>> 
>> http://buildbot.net/buildbot/docs/0.8.6p1/release-notes.html
>> "Forced builds now require that a ForceScheduler be defined in the Buildbot 
>> configuration."
>> 
>> It can be solved simple with adding ForceScheduler. I'm going
>> to file a bug report and prepare a patch to fix it soon.
>> 
>> br,
>> Ossy
>> ___
>> 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

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Can we distinguish imported tests in LayoutTests/css3 ?

[Darin Adler]

On Jun 12, 2012, at 10:17 AM, Ryosuke Niwa wrote:

> The last time we had this discussion, the consensus appeared to be renaming 
> it to RegressionTests.

I still like that name.

> Are you suggesting that we have LayoutTests/W3C?

Yes. Something like RegressionTests/ImportedTests/W3C.

> we have at least 15 test running scripts

Too many!

-- Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] removing js-test-post.js WAS: Can we distinguish imported tests in LayoutTests/css3 ?

[Alexey Proskuryakov]

12.06.2012, в 10:31, Ojan Vafai написал(а):

> I'm sympathetic to this argument. In this case, the only way it makes the 
> tests more fragile is that it makes them depend on us firing the onload 
> event. I feel like so many things would break if you break onload, that it's 
> not really making the tests more fragile in practice. 

There are always edge cases that are complicated. No longer than yesterday, I 
was helping someone with a test that mysteriously didn't print "Test Complete" 
in ToT WebKit, while that worked in Firefox. Unfortunately, I didn't capture 
the test back then, and cannot reproduce the problem right now (we ended up 
using a whole different approach in that test).

Anyway, it's not difficult to think of cases where onload timing gets tricky.

> Increasingly, js-test-pre.js is not being used by autogenerated code. This is 
> something we should encourage.

I'm not sure what you mean by this. The make-new-script-test tool includes 
js-test-pre.js and js-test-post.js, and it's the only way to autogenerate the 
boilerplate that I'm aware of.

- WBR, Alexey Proskuryakov

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] removing js-test-post.js WAS: Can we distinguish imported tests in LayoutTests/css3 ?

[Ojan Vafai]

On Tue, Jun 12, 2012 at 10:18 AM, Alexey Proskuryakov  wrote:

>
> 12.06.2012, в 9:59, Ojan Vafai написал(а):
>
> One more item I'd add to this list:
> -Replace js-test-post.js with an onload handler in js-test-pre.js so we
> can get rid of that file entirely.
>
>
> I think that this is a separate discussion to test directory
> reorganization.
>

Makes sense. Changed the subject. Seemed related to me in that it might
simplify moving tests that include it.


> Also, I'd question that "getting rid" of some boilerplate in autogenerated
> code is a worthy goal, given that it will make tests more fragile.
>

I'm sympathetic to this argument. In this case, the only way it makes the
tests more fragile is that it makes them depend on us firing the onload
event. I feel like so many things would break if you break onload, that
it's not really making the tests more fragile in practice.

Increasingly, js-test-pre.js is not being used by autogenerated code. This
is something we should encourage. By standardizing on this for most
dump-as-text tests, we simplify test code and simplify making sense of test
failures (e.g. you don't need to understand the custom test harness in the
test to figure out a failure). We should only use autogenerated
script-tests for cases where we need to isolate out the pure JS code (e.g.
pure JS tests or tests that need to be run both inside and outside a
Worker) since they add the extra overhead of dealing with an extra file and
need the extra work of running the autogeneration script.

In practice, what happens right now is that people often leave out
js-test-post.js and you lose a significant bit of the test harness's
functionality (i.e. checking for unintentional parse errors).

The benefits far outweigh the risks here IMO.

Ojan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Can we distinguish imported tests in LayoutTests/css3 ?

[Alexey Proskuryakov]

12.06.2012, в 9:59, Ojan Vafai написал(а):

> One more item I'd add to this list:
> -Replace js-test-post.js with an onload handler in js-test-pre.js so we can 
> get rid of that file entirely.


I think that this is a separate discussion to test directory reorganization.

Also, I'd question that "getting rid" of some boilerplate in autogenerated code 
is a worthy goal, given that it will make tests more fragile.

- WBR, Alexey Proskuryakov

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Can we distinguish imported tests in LayoutTests/css3 ?

[Ryosuke Niwa]

On Tue, Jun 12, 2012 at 9:50 AM, Darin Adler  wrote:
>
> - Each time we change the tests we need to make sure we haven’t broken new
> tests on various platforms and that we’ve updated the skipped or test
> expectations files properly.
>

- When rearranging tests that share the test shell JavaScript files we’d
> likely need to update relative paths. Might also be nice to put that test
> shell in a more logical location.


For these 2 points, we probably need some script to move a test or a set of
tests.


> - Calling the entire directory that hosts our regression test suite
> LayoutTests seems wrong; that name was apropos when most of the tests were
> for layout issues, very early in the life of the test machinery.


The last time we had this discussion, the consensus appeared to be renaming
it to RegressionTests. Do people have different opinions at this time?

- Imported test suites ought to be gathered together in a single directory
> then grouped by where they were imported from by having a directory for
> each source.
>

Are you suggesting that we have LayoutTests/W3C?

- We want to continue the practice of keeping copies imported test suites
> as close to the original as possible. In some cases in the past that meant
> preserving incorrect imported tests with expected “failures” that weren’t
> really failures. Often we’d put corrected copies elsewhere in the test tree
> while waiting for or working to get the imported test to fixed at the
> source.
>
> - Keeping the tests that require the HTTP server in a separate directory
> still seems like it might serve a purpose, although there seems to be an
> extra directory level in the test hierarchy there that is not helpful.
>

- Making JavaScript-only tests runnable without WebKit’s involvement would
> be a welcome feature; great for anyone working on JavaScriptCore. That
> means we’d want to be sure to put those into their own directories. Ideally
> that would eventually be coupled by a project to move the Mozilla test
> suite currently in JavaScriptCore into the main WebKit regression tests.
>

Another point. Related to Oliver's response:
http://lists.webkit.org/pipermail/webkit-dev/2011-September/017872.html on
September 8th, 2011, we have at least 15 test running scripts (the number
has since increased :( ) at this point:

   1. run-api-tests
   2. run-binding-tests
   3. run-chromium-webkit-unit-tests
   4. run-gtk-tests
   5. run-iexploder-tests
   6. run-javascriptcore-tests
   7. run-mangleme-tests
   8. run-perf-tests
   9. run-qtwebkit-tests
   10. run-regexp-tests
   11. run-test-webkit-api # what's the difference between this and
   run-api-tests?
   12. test-webkit-scripts
   13. run-webkit-tests
   14. test-webkitpy
   15. test-webkitperl

And I find it really annoying that it's non-trivial to figure out which
script runs which tests and where the test code is located. Some of these
test runners appear to be port specific while others seem to be
port-agnostic.

Also, given that the vast majority of these test runners also run some sort
of regression tests (e.g. run-api-tests; API tests certainly are regression
tests for some APIs or so I hope), renaming LayoutTests to RegressionTests
might be a little confusing because of that.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Can we distinguish imported tests in LayoutTests/css3 ?

[Ojan Vafai]

On Tue, Jun 12, 2012 at 9:50 AM, Darin Adler  wrote:

> On Jun 11, 2012, at 7:33 PM, Mike Lawther wrote:
>
> > Does having the 'fast' directory still serve a useful purpose?
>
> Not sure it does.
>
> The original intent was to put more extensive complete tests that were
> slow but had greater coverage outside the “fast” directory, so that you
> could run the majority of the most useful tests in a few seconds.
>
> Times to run tests have increased so much, despite the parallel test
> runner, that it’s no longer a useful distinction.
>
> A new logical organization for tests would be welcome, and it’s a big
> project where I am not sure we have a clear enough plan to start on it
> incrementally. Here are a few considerations:
>
> - Moving test files around could cause some huge churn, with some
> super-slow check-outs for everyone working on the project and many old
> patches potentially needing to be rebased. This is especially true when
> image results are involved.
>
> - Each time we change the tests we need to make sure we haven’t broken new
> tests on various platforms and that we’ve updated the skipped or test
> expectations files properly.
>

If we could get more EWS bots running the tests (currently only Chromium
Linux does), that would make this a lot easier. It would also improve
general development on WebKit. Fewer cases of people being surprised by
broken layout tests when a patch gets committed.

One more item I'd add to this list:
-Replace js-test-post.js with an onload handler in js-test-pre.js so we can
get rid of that file entirely.


> - Calling the entire directory that hosts our regression test suite
> LayoutTests seems wrong; that name was apropos when most of the tests were
> for layout issues, very early in the life of the test machinery.
>
> - Imported test suites ought to be gathered together in a single directory
> then grouped by where they were imported from by having a directory for
> each source.
>
> - We want to continue the practice of keeping copies imported test suites
> as close to the original as possible. In some cases in the past that meant
> preserving incorrect imported tests with expected “failures” that weren’t
> really failures. Often we’d put corrected copies elsewhere in the test tree
> while waiting for or working to get the imported test to fixed at the
> source.
>
> - Keeping the tests that require the HTTP server in a separate directory
> still seems like it might serve a purpose, although there seems to be an
> extra directory level in the test hierarchy there that is not helpful.
>
> - When rearranging tests that share the test shell JavaScript files we’d
> likely need to update relative paths. Might also be nice to put that test
> shell in a more logical location.
>
> - Making JavaScript-only tests runnable without WebKit’s involvement would
> be a welcome feature; great for anyone working on JavaScriptCore. That
> means we’d want to be sure to put those into their own directories. Ideally
> that would eventually be coupled by a project to move the Mozilla test
> suite currently in JavaScriptCore into the main WebKit regression tests.
>
> -- Darin
> ___
> 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] Can we distinguish imported tests in LayoutTests/css3 ?

[Darin Adler]

On Jun 11, 2012, at 7:33 PM, Mike Lawther wrote:

> Does having the 'fast' directory still serve a useful purpose?

Not sure it does.

The original intent was to put more extensive complete tests that were slow but 
had greater coverage outside the “fast” directory, so that you could run the 
majority of the most useful tests in a few seconds.

Times to run tests have increased so much, despite the parallel test runner, 
that it’s no longer a useful distinction.

A new logical organization for tests would be welcome, and it’s a big project 
where I am not sure we have a clear enough plan to start on it incrementally. 
Here are a few considerations:

- Moving test files around could cause some huge churn, with some super-slow 
check-outs for everyone working on the project and many old patches potentially 
needing to be rebased. This is especially true when image results are involved.

- Each time we change the tests we need to make sure we haven’t broken new 
tests on various platforms and that we’ve updated the skipped or test 
expectations files properly.

- Calling the entire directory that hosts our regression test suite LayoutTests 
seems wrong; that name was apropos when most of the tests were for layout 
issues, very early in the life of the test machinery.

- Imported test suites ought to be gathered together in a single directory then 
grouped by where they were imported from by having a directory for each source.

- We want to continue the practice of keeping copies imported test suites as 
close to the original as possible. In some cases in the past that meant 
preserving incorrect imported tests with expected “failures” that weren’t 
really failures. Often we’d put corrected copies elsewhere in the test tree 
while waiting for or working to get the imported test to fixed at the source.

- Keeping the tests that require the HTTP server in a separate directory still 
seems like it might serve a purpose, although there seems to be an extra 
directory level in the test hierarchy there that is not helpful.

- When rearranging tests that share the test shell JavaScript files we’d likely 
need to update relative paths. Might also be nice to put that test shell in a 
more logical location.

- Making JavaScript-only tests runnable without WebKit’s involvement would be a 
welcome feature; great for anyone working on JavaScriptCore. That means we’d 
want to be sure to put those into their own directories. Ideally that would 
eventually be coupled by a project to move the Mozilla test suite currently in 
JavaScriptCore into the main WebKit regression tests.

-- Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] SVG video chat (2)

[Philip Rogers]

Last month we had a video chat with the SVG team in WebKit and it was very
well received. If there are other highly-coupled, highly-distributed
sub-components of WebKit I would highly recommend setting one of these up.


SVG,

The last video chat went great and as a group we decided to do it again. If
you'd like to join please add your name, availability, and topics you'd
like to discuss to the spreadsheet:
*
https://docs.google.com/spreadsheet/ccc?key=0AorN2u5hCJiUdDQ3RUM0bzlGZUpENnAtTHh5aFJBU0E
*

Thanks,
Philip
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] DOM tree traversal on detached nodes

[Filip Pizlo]

On Jun 11, 2012, at 11:35 PM, Ojan Vafai  wrote:

> On Mon, Jun 11, 2012 at 9:32 PM, Filip Pizlo  wrote:
> I think that a lot of the performance penalty can be alleviated by:
> 
> 1) Moving rarely executed paths into non-inlineable methods.
> 
> 2) Marking the various refing methods ALWAYS_INLINE, after you've moved as 
> much as possible out of line.
> 
> 3) Using LIKELY and UNLIKELY where appropriate.
> 
> We should only add these when we can point to a measurable benefit IMO (maybe 
> that's what you meant by "appropriate"?). My experience is that marking 
> codepaths LIKELY/UNLIKELY very rarely has a measurable performance 
> improvement with modern compilers + x86 processors. I can't speak for arm 
> since I have less direct experience optimizing for arm.

It's more for the benefit of the compiler than the hardware. It can, for 
example, improve register allocation if the compiler knows that in case of 
interference between variables used predominantly on cold paths and variables 
used predominantly on hot paths, it should spill the cold ones. The following 
is a good example:

x = some expression;
if (--thingy) doStuff(); // not inlineable
use x;

The function call will interfere with all caller-save registers. Which is most 
registers. The compiler can do one of three things: (1) spill x, (2) put x in 
callee-save register, of which there is a limited supply, or (3) put x in a 
caller save but insert save/restore code around the doStuff() call. Last I 
checked, gcc would prefer (2) or (1) if it ran out of registers unless it knew 
that doStuff() was called rarely. Hence the utility of using UNLIKEY in this 
case. If you know that it is unlikely then you want the compiler to go with 
option (3), which greatly relieves register pressure at the cost of making the 
rare call slightly slower. 

You're right that it's not a huge win or even a measurable win in many cases.  
The conditions have to be just right - for one it has to be a case where the 
compiler wasn't already smart enough to do the right thing.  And current 
compilers are quite smart indeed so the benefits of these macros will converge 
to zero over time. 

In my experience it's a benefit in inline functions that get called from many 
places, since after a lot of inlining the compiler can use all the help it can 
get in trying to unravel which parts of the code matter and which don't. I've 
seen it result in a 5% progression in the past, though admittedly it was in 
runtime functions that get inlined *everywhere* like allocation and GC write 
barriers.

In this particular case, I have a hunch that these macros might make a 
difference. It's worth a shot since adding them doesn't require much effort. 

> 
> The reason I suggest these things is that you're adding a lot of code in a 
> bunch of methods, but a lot of that logic looks like it will execute 
> infrequently. That reduces inlining opportunities. And in places where the 
> methods are still inlined, you're adding code bloat that reduces icache 
> locality and may blow out branch prediction caches.
> 
> I'm also not sure that your benchmark is conclusive. What does the perf test 
> have in common with things we know we care about, like say PLT?
> 
> -Filip
> 
> On Jun 11, 2012, at 8:45 PM, Kentaro Hara  wrote:
> 
> > Thanks ggaren!
> >
> > I filed a bug here: https://bugs.webkit.org/show_bug.cgi?id=88834
> >
> >
> >> I believe you could achieve (a) (i.e., preserve all reachable nodes 
> >> without help from the JavaScript garbage collector) with these semantics 
> >> in the C++ DOM:
> >>
> >> = Design =
> >>
> >> ref():
> >>
> >> ++this->refCount
> >> ++root()->refCount
> >>
> >> deref():
> >>
> >> --this->refCount
> >> --root()->refCount
> >
> > Actually I've tried the approach yesterday but confirmed 25.9%
> > performance regression, even if I have TreeShared hold a pointer to
> > the root. Please look at the WIP patch and the performance test in
> > https://bugs.webkit.org/show_bug.cgi?id=88834.
> >
> > What I've learned is that we must not insert any line to ref() and
> > deref(), which are performance sensitive:)
> >
> > I am now trying another approach that hacks Node destruction. Let's
> > discuss the implementation details in the bug.
> >
> > Thanks!
> >
> >
> > On Tue, Jun 12, 2012 at 12:18 PM, Geoffrey Garen  wrote:
> >>
> >> IMHO, (a) would be the best semantics.
> >>
> >>
> >> I agree, and I think that the specification actually does require this.
> >>
> >> The real issue here is how to fix this bug efficiently. I believe that 
> >> Geoff Garen has been contemplating this and also has a proposal for how to 
> >> do it.
> >>
> >>
> >> I believe you could achieve (a) (i.e., preserve all reachable nodes 
> >> without help from the JavaScript garbage collector) with these semantics 
> >> in the C++ DOM:
> >>
> >> = Design =
> >>
> >> ref():
> >>
> >> ++this->refCount
> >> ++root()->refCount
> >>
> >> deref():
> >>
> >> --this->refCount
> >> --root()->refCount

Re: [webkit-dev] No (was Re: can we stop using Skipped files?)

[Ryosuke Niwa]

On Mon, Jun 11, 2012 at 11:58 PM, Dirk Pranke  wrote:

> On Mon, Jun 11, 2012 at 11:45 PM, Ryosuke Niwa  wrote:
> > On Mon, Jun 11, 2012 at 11:37 PM, Dirk Pranke 
> wrote:
> >>
> >> On Thu, Jun 7, 2012 at 4:47 PM, Dirk Pranke 
> wrote:
> >> > I believe most if not all of the ports have started using either
> >> > TestExpectations files or a combination of TestExpectations files
> >> > (except for the Apple Win port).
> >> >
> >> > Can we explicitly switch to the TestExpectations files at this point
> >> > and drop support for Skipped files on the other ports (and perhaps
> >> > disable old-run-webkit-tests for all but apple win)?
> >> >
> >> > If not, what needs to happen so that we can do that? Do we need to
> >> > land more of the discussed changes to the syntax of the
> >> > TestExpectations file? Anything else?
> >> >
> >> > It would be nice to get rid of the complexity (both in the code and in
> >> > our heads) if we can.
> >> >
> >>
> >> It is clear that we're not at the point where we can stop using
> >> Skipped files yet. There appear to be at least two things that need to
> >> be done, before we reach that point, let alone remove ORWT (which is
> >> clearly right off the table right now).
> >>
> >> First, NRWT appears to still be missing some functionality, and that
> >> is keeping some people on ORWT. I was not aware of this, so thanks to
> >> those of you that have pointed this out. Please keep the bugs and
> >> feature requests coming! I have filed bug 88680 to track such issues.
> >>
> >> Second, it appears that some people feel strongly that NRWT "less
> >> hackable" than ORWT, and that this is a significant issue for them. I
> >> was not aware of this, at least not that it was a significant issue. I
> >> have filed bug 88841 to track any suggestions or improvements people
> >> might have for fixing this.
> >>
> >> Given the above, it seems that we can't yet switch to TestExpectations
> >> files, but ...
> >>
> >> There is another option: we could change ORWT to read the
> >> TestExpectations files instead and just honor the SKIP entries
> >> (ignoring the other expectations). Is that something we should
> >> consider?
> >
> >
> > That seems like a nice intermediate step so that people don't have to
> > maintain both files. I would aruge, however, that we should simplify the
> > syntax for SKIP entries (it should no mean block the work to make ORWT
> > support TestExpectations but it's probably easier to simplify the syntax
> > before ORWT supports it).
> >
>
> Haven't we already "aruge"d this? :) I thought we already agreed that
> we want to change the syntax in the file (cf. bug 86796)?


Right. I'm just suggesting that we should fix that bug first.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev