Bug 140914 – [Win] Unable to reliably run tests in parallel

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Brent Fulgham]

Whenever I increase the number of shards on our Windows tests bots above one I encounter variation in test failures and results. There seems to be some kind of cross-talk between tests that prevent us from running more jobs in parallel.

We need to fix this to improve our test throughput.

[Radar WebKit Bug Importer]

<rdar://problem/19609178>

[Adam Roben (:aroben)]

Could the different test executables be stomping on each other's preferences? A number of tests change particular preferences while they run.

[Brent Fulgham]

(In reply to comment #2)
> Could the different test executables be stomping on each other's
> preferences? A number of tests change particular preferences while they run.

Definitely. However, I tried to account for this in "libraryPathForDumpRenderTree()", which is based on the Mac implementation. It looks for DUMPRENDERTREE_TEMP in the environment, and uses that as the basis for its local preference storage. This is supposed to be unique per running test instance.

I guess the first thing to check is that this is unique for each running test shard. If they are not, that might explain the problem.

[Brent Fulgham]

(In reply to comment #3)
> (In reply to comment #2)
> > Could the different test executables be stomping on each other's
> > preferences? A number of tests change particular preferences while they run.
> 
> Definitely. However, I tried to account for this in
> "libraryPathForDumpRenderTree()", which is based on the Mac implementation.
> It looks for DUMPRENDERTREE_TEMP in the environment, and uses that as the
> basis for its local preference storage. This is supposed to be unique per
> running test instance.
> 
> I guess the first thing to check is that this is unique for each running
> test shard. If they are not, that might explain the problem.

So I added a printf to the libraryPathForDumpRenderTree, and confirmed that each shard is getting its own temporary directory, which it uses to seed the following preferences:

    CFPreferencesSetAppValue(WebDatabaseDirectoryDefaultsKey, WebCore::pathByAppendingComponent(libraryPath, "Databases").createCFString().get(), kCFPreferencesCurrentApplication);
    CFPreferencesSetAppValue(WebStorageDirectoryDefaultsKey, WebCore::pathByAppendingComponent(libraryPath, "LocalStorage").createCFString().get(), kCFPreferencesCurrentApplication);
    CFPreferencesSetAppValue(WebKitLocalCacheDefaultsKey, WebCore::pathByAppendingComponent(libraryPath, "LocalCache").createCFString().get(), kCFPreferencesCurrentApplication);

The question, is whether CFPreferencesSetAppValue is sticking these values in the exact same place, causing all the different shards to see the same values.

[Adam Roben (:aroben)]

(In reply to comment #4)
> The question, is whether CFPreferencesSetAppValue is sticking these values
> in the exact same place, causing all the different shards to see the same
> values.

I'm pretty sure it does.

You'll probably also need to audit uses of localUserSpecificStorageDirectory() and roamingUserSpecificStorageDirectory().

[Brent Fulgham]

(In reply to comment #5)
> (In reply to comment #4)
> > The question, is whether CFPreferencesSetAppValue is sticking these values
> > in the exact same place, causing all the different shards to see the same
> > values.
> 
> I'm pretty sure it does.
> 
> You'll probably also need to audit uses of
> localUserSpecificStorageDirectory() and
> roamingUserSpecificStorageDirectory().

After talking with a few people, I think it may NOT be possible to do this on Windows with CoreFoundation. The CFPreference store is going to end up being shared between the different processes.

[Adam Roben (:aroben)]

Maybe you could give each process a unique CFBundle name? That should give them separate preferences.

[Per Arne Vollan]

Created attachment 285319 [details]
Patch

[Per Arne Vollan]

There might be additional issues we need to fix, but this patch fixes the failures I see when running the dom tests in parallel.

[Brent Fulgham]

Comment on attachment 285319 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285319&action=review

Fantastic! r=me

> Tools/Scripts/webkitpy/common/system/path.py:58
> +        path = re.sub('/cygdrive/c', 'c:', path)

I think this would be better if we could capture the drive letter and pass it to the substitution so that "/cygdrive/d' -> 'd:'.

This would be fine to do as a follow-up to this change.

[WebKit Commit Bot]

Comment on attachment 285319 [details]
Patch

Clearing flags on attachment: 285319

Committed r204123: <http://trac.webkit.org/changeset/204123>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 160597

[Per Arne Vollan]

Created attachment 285424 [details]
Patch

[Brent Fulgham]

Comment on attachment 285424 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285424&action=review

Thanks for improving the change! I'm not sure what the cause of the timeout was that you fixed this time around, but these changes look good on their own merits. If the tests pass please land it.

> Tools/Scripts/webkitpy/common/system/path.py:67
> +                fileSystemRoot = _CygPath.convert_using_singleton(match.group(1))

Is it possible that someone might run tools from one root (e.g., 'C:'), and also run separate tools from another root (e.g., 'D:') during the same tool run? I guess that's pretty unlikely.

[Per Arne Vollan]

Created attachment 285564 [details]
Patch

[Per Arne Vollan]

(In reply to comment #15)
> Comment on attachment 285424 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=285424&action=review
> 
> Thanks for improving the change! I'm not sure what the cause of the timeout
> was that you fixed this time around, but these changes look good on their
> own merits. If the tests pass please land it.
> 
> > Tools/Scripts/webkitpy/common/system/path.py:67
> > +                fileSystemRoot = _CygPath.convert_using_singleton(match.group(1))
> 
> Is it possible that someone might run tools from one root (e.g., 'C:'), and
> also run separate tools from another root (e.g., 'D:') during the same tool
> run? I guess that's pretty unlikely.

Thanks for reviewing :) I updated the patch to handle this case.

[Per Arne Vollan]

Created attachment 285826 [details]
Patch

[Brent Fulgham]

Comment on attachment 285826 [details]
Patch

Excellent! r=me.

[WebKit Commit Bot]

Comment on attachment 285826 [details]
Patch

Clearing flags on attachment: 285826

Committed r204376: <http://trac.webkit.org/changeset/204376>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.