I think the desired feature is to have 'isolate batcharchive' to return the individual items that were cold in the cache.

Do not look at the tasks' performance_stats, since it's influenced by other factors like VM recycling.

https://go.chromium.org/luci/client/cmd/isolate

> Do not look at the tasks' performance_stats, since it's influenced by other factors like VM recycling.

I'll see only the number of duplicated tasks and total tasks.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/36541eb81aae62b267dcc88d1fdaef912450299a

commit 36541eb81aae62b267dcc88d1fdaef912450299a
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Aug 01 02:09:56 2018

Support dummy commit for LASTCHANGE

This is to generate reproducible binary between commits for Linux by giving use_dummy_lastchange=true in args.gn.

This CL makes binaries independent from commit hash at least for below (time consuming) targets on Linux.
* content_unittests
* interactive_ui_tests
* content_shell (for webkit_layout_tests)
* content_browsertests
* browser_tests

If we can generate deterministic binary, test execution on build bot can be cached. And it will improve CQ cycle time.
But we don't know how often we make changes not introducing functional change in binaries.
So this CL is mainly for getting some stats from test on Linux buildbot and evaluate whether it is better to go forward or not for other platforms.

See more backgrounds here.
https://docs.google.com/document/d/16dalG0ssugZlwc7BBzUoBQUmuqM0S_gwtG2uxL0htdw/edit#

TBR: pfeldman@chromium.org
Bug: 869348
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I1b351b2135fd5e749ed575484b2182e847a420c9
Reviewed-on: https://chromium-review.googlesource.com/1151169
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579668}
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/base/BUILD.gn
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/build/.gitignore
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/build/mac/tweak_info_plist.gni
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/build/util/BUILD.gn
[add] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/build/util/LASTCHANGE.dummy
[add] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/build/util/lastchange.gni
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/chrome/chrome_cleaner/constants/BUILD.gn
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/chrome/common/BUILD.gn
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/chrome/installer/linux/BUILD.gn
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/chrome/process_version_rc_template.gni
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/chrome/test/chromedriver/BUILD.gn
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/components/cronet/android/BUILD.gn
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/components/cronet/ios/BUILD.gn
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/components/version_info/BUILD.gn
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/extensions/shell/BUILD.gn
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/extensions/shell/installer/linux/BUILD.gn
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/headless/BUILD.gn
[modify] https://crrev.com/36541eb81aae62b267dcc88d1fdaef912450299a/remoting/host/win/BUILD.gn

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/23f7bc518a817b8c102fe144e87b932a273ce053

commit 23f7bc518a817b8c102fe144e87b932a273ce053
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Aug 01 13:12:50 2018

Enable use_dummy_lastchange=true for linux_chromium_rel_ng builder

This is to take stats of cache hit rate on swarming test execution.

I will record stats to the issue few days later.

I changed dummy value due to some test failure.

Bug: 869348
Change-Id: Ied830d0e783065e3aad48f066ad6b471b735fe00
Reviewed-on: https://chromium-review.googlesource.com/1157737
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579787}
[modify] https://crrev.com/23f7bc518a817b8c102fe144e87b932a273ce053/build/util/LASTCHANGE.dummy
[modify] https://crrev.com/23f7bc518a817b8c102fe144e87b932a273ce053/tools/mb/mb_config.pyl

Hmm, no deduplication happened for out main target tests so far.

From 24 hour stats,
* browser_tests: 0 of 8360 tasks were deduplicated
https://chromium-swarm.appspot.com/tasklist?c=name&c=state&c=created_ts&c=duration&c=pending_time&c=pool&c=bot&et=1533253440000&f=buildername-tag%3Alinux_chromium_rel_ng&f=stepname-tag%3Abrowser_tests%20(with%20patch)&l=50&s=created_ts%3Adesc&st=1533167040000
* content_unittests: 0 of 677 tasks were deduplicated
https://chromium-swarm.appspot.com/tasklist?c=name&c=state&c=created_ts&c=duration&c=pending_time&c=pool&c=bot&et=1533253440000&f=buildername-tag%3Alinux_chromium_rel_ng&f=stepname-tag%3Acontent_unittests%20(with%20patch)&l=50&s=created_ts%3Adesc&st=1533167040000
* interactive_ui_tests: 0 of 2466 tasks were deduplicated
https://chromium-swarm.appspot.com/tasklist?c=name&c=state&c=created_ts&c=duration&c=pending_time&c=pool&c=bot&et=1533253440000&f=buildername-tag%3Alinux_chromium_rel_ng&f=stepname-tag%3Ainteractive_ui_tests%20(with%20patch)&l=50&s=created_ts%3Adesc&st=1533167040000
* content_browsertests: 0 of 675 tasks were deduplicated
https://chromium-swarm.appspot.com/tasklist?c=name&c=state&c=created_ts&c=duration&c=pending_time&c=pool&c=bot&et=1533253440000&f=buildername-tag%3Alinux_chromium_rel_ng&f=stepname-tag%3Acontent_browsertests%20(with%20patch)&l=50&s=created_ts%3Adesc&st=1533167040000

For all tests in linux_chromium_rel_ng, 3817 out of 126633 tasks (3%) were deduplicated.
https://chromium-swarm.appspot.com/tasklist?c=name&c=state&c=created_ts&c=duration&c=pending_time&c=pool&c=bot&et=1533253440000&f=buildername-tag%3Alinux_chromium_rel_ng&l=50&s=created_ts%3Adesc&st=1533167040000

This is bit improved from the stats in a week ago, 2477 out of 107144 tasks (2.3%) were deduplicated.
https://chromium-swarm.appspot.com/tasklist?c=name&c=state&c=created_ts&c=duration&c=pending_time&c=pool&c=bot&et=1532648640000&f=buildername-tag%3Alinux_chromium_rel_ng&l=50&s=created_ts%3Adesc&st=1532562240000

So, My conclusion is, current test binaries almost always have different functionality.
And we won't get large caching benefit from current effort on deterministic build.

Hm, that's a bit surprising to me. Have you tried sending two try jobs for one CL that e.g. adds a global -W flag (say -w to disable all warnings) and then confirmed that it gets cached test output? (the warning flag shouldn't change output, and sending the two CLs in succession makes it hopefully likely that they get built at a similar point in time, so that at least base_unittests maybe gets a hit)

#6, I will confirm that.

But it may be yet reasonable for me that (patch content + latest commit) rarely become idential to the previous ones.
And maybe this low cache hit rate proved that GN's dependency analysis is working well.

Testing.
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_rel_ng/155913
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_rel_ng/155914

#6

Bit sadly, I failed to get same base commit for the jobs in #8
ec7de27434ba765f251db0ea837a354e4da8f6c5 vs 9924c89b63a13773747d186cf2fe7066b02eb747

But that should not affect browser_tests on linux_chromium_rel_ng builder.
https://chromium.googlesource.com/chromium/src.git/+log/ec7de27434ba765f251db0ea837a354e4da8f6c5


I compared isolated files.
https://isolateserver.appspot.com/browse?namespace=default-gzip&hash=da824eec0652224e3bc5d008ff339cae5d433f6f
https://isolateserver.appspot.com/browse?namespace=default-gzip&hash=71701b0e0925fddcc8d497bb1ec7495b73146f1c

And found that there are some differences in the files.
* 3 of .<hash>.tar files 
* out/Release/browser_tests, I guess this is come from difference of libGLESv2.so
* out/Release/libGLESv2.so, why we send .so files for non-component-build test?
  Due to dynamic loading?
  https://cs.chromium.org/chromium/src/third_party/swiftshader/src/OpenGL/libGLESv2/libGLESv2.hpp?l=308&rcl=9e22c542d6be9bdc4066a224264b9d3a1fb73018
* out/Release/remoting-webapp.v2.zip, difference of zip timestamp?
* out/Release/v8_context_snapshot.bin

I will file separate issue for above files.

#9
> * 3 of .<hash>.tar files 

Not 3 files, actually only .52c202fcfcc0af7ed96009b0f47b94acdff6c21d.tar and .dc1705f0255e83b74640d14bec8290e12e2fd61e.tar is different.

And its difference looks come from
$ diff -r dc 52
Only in dc/out/Release/fontconfig_caches: 40f35dcc-ce47-4fd7-bcad-f33e4ee1dfba-le64.cache-7
Only in 52/out/Release/fontconfig_caches: dcc09cc5-1256-4419-a29b-3095bab228dd-le64.cache-7

Looks like fontconfig_caches is a significant bug. I suspect it hasn't been caught by the deterministic builder because it's not a binary file and the tooling didn't check for other files with unknown extension.

Any particular reason that the deterministic builder doesn't check for all files, rather than just executable files?

"We had to start somewhere", aka narrowing the scope. But then it never been followed through.

Hmm, how do we choose files included in isolated files?
I guess buildbot has yet some non-deterministic files not listed as outputs in BUILD.gn and those are included in isolated files.

So I want to run "inverted clean" in buildbot. crbug.com/852926

Maybe instead of comparing all files, we can compare just all files that end up in isolates and are sent to swarming. Opening all the out/gn/*.isolate files (they are json except with '' instead of "" strings) and looking at $.variables.files finds those.

(Then we only need to worry about stale .isolate files hanging around, but these are rare, and we could `rm out/gn/*.isolate` early in a build if we don't yet.)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2567b5adc6258e7ce0848e4e85970394a7c8c218

commit 2567b5adc6258e7ce0848e4e85970394a7c8c218
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Mon Aug 06 12:26:04 2018

Compare more build artifacts

Let me add more files to be checked for test execution caching on swarming.

Linux deterministic builder is only the tree closer, so I updated whitelist of linux only in this CL.

Bug: 869348
Change-Id: I390af4d5899acb7dc2a3f311870b135e036e4854
Reviewed-on: https://chromium-review.googlesource.com/1163583
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580852}
[modify] https://crrev.com/2567b5adc6258e7ce0848e4e85970394a7c8c218/tools/determinism/compare_build_artifacts.py
[modify] https://crrev.com/2567b5adc6258e7ce0848e4e85970394a7c8c218/tools/determinism/deterministic_build_whitelist.pyl

#21,
Ah, then we want to stop using directory deps for fontconfig_caches.

Hm yes, having a directory dep with a generated dir is a problem if the files in that dir change from time to time: swarming will always send all files in the dir, so possibly old stale files too. For fontconfig, the easiest fix is probably to make 
base/test/generate_fontconfig_caches.cc delete the contents of the cache directory before it does anything else.

I compared isolate inputs for browser_tests again.
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_rel_ng/159263
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_rel_ng/159265

Difference is looks like
$ diff -r 16 bb
Only in 16/out/Release/pyproto/components: image_fetcher
Only in bb/out/Release/pyproto/net/third_party/quic/core/proto: quic_trace_pb2.py
Only in bb/out/Release/pyproto/third_party/blink/renderer/platform/scheduler/base/proto: task_queue_manager_test_description_pb2.py

Dependency to pytproto directory in BUILD.gn is not good.
https://cs.chromium.org/chromium/src/chrome/test/BUILD.gn?l=1074&rcl=5650da8d48859d1097f8917e16719b4cc694ef83

Some builder VMs leave temporary build outputs, but some others do not.
And such difference causes difference of isolate inputs.

And even if we are using directory specification in BUILD.gn, "gn inverted clean" won't help.

For long term solution, we need to stop using directory dependency in BUILD.gn.
For short term solution, I'd like to land something like landmine change for buildbot.

> For long term solution, we need to stop using directory dependency in BUILD.gn.

For something like webkit_layout_tests, we depend on //third_party/WebKit/LayoutTests/, and there are tens of thousands of files under that directory. Trying to list every file would be a serious headache, and would probably also significantly slow down GN. 

We thought about this a bunch when first switching to GN, and didn't see a good way to do this, so I'm not optimistic that we can fix it now, but feel free to investigate it, maybe you'll come up with something we didn't see or ruled out too early.

Doing something like a landmine change would cause you to have to do a full rebuild; it seems like that would be more expensive than whatever savings you'd get back by having a deterministic isolate?

I'd be surprised if the problem is a temporary build output that varied from builder to builder. I suspect it's more likely something left over from a previous build, either where we reused the build directory from a different configuration, or simply because files were deleted or renamed or something. Unless maybe that's what you meant?

However, I'm not sure if I see the need to have a dependency on a directory underneath root_build_dir. It seems like in most cases there you should be able to specify the list of files exactly, because they'd all be being generated.

[ That might not work for cases where we copy whole directories from the source tree, but I don't know how common that is outside of mac frameworks and bundles ].

> For something like webkit_layout_tests, we depend on //third_party/WebKit/LayoutTests/, and there are tens of thousands of files under that directory. Trying to list every file would be a serious headache, and would probably also significantly slow down GN. 

We can ignore directory managed by git.

> Doing something like a landmine change would cause you to have to do a full rebuild; it seems like that would be more expensive than whatever savings you'd get back by having a deterministic isolate?

Yes, cleanup every time is expensive, but I thought to do such things once per a day on builders.


Anyway, I will specify output files explicitly in dependency. Will confirm that is sufficient or not.

#26,

I compared isolate input for webkit_layout_test too.
It contains difference of many .pyc files and .git dir in third_party/pywebsocket/src/
https://pastebin.com/v3K5UFJk

#29,

For .pyc files, we need to 
* set `sys.dont_write_bytecode = True` in relevant python scripts or
* set PYTHONDONTWRITEBYTECODE=1 envvar when building on buildbot or
* use python3.7 https://www.python.org/dev/peps/pep-0552/ or
* ignore the files in isolate client

maruel@, can we use filtering for isolate inputs?

#30,

isolate client seems to have such future, but does not work as expected?

https://cs.chromium.org/chromium/infra/luci/client/isolateserver.py?l=81&rcl=db606474e98248472fbfc9a2278f94380e31c07d

#31, Filed 872245

Do you have a list of the problematic directory dependencies? (or just attach the file lists in both isolates)

#34,
I don't have such a list.

About skipping .pyc files:

https://cs.chromium.org/chromium/infra/go/src/go.chromium.org/luci/client/cmd/isolate/common.go?g=0&l=92

and

https://cs.chromium.org/chromium/infra/go/src/go.chromium.org/luci/client/isolate/isolate.go?g=0&l=86

We use the Go implementation on the recipe side, the python one on the bot side.

#36
Do you know why webkit_layout_test contains many .pyc files in isolate inputs?

I wasn't personally involved in this (and haven't looked right now) so I don't know, probably a silly oversight.

Re comment 26, 35: As said elsewhere, the only problem with data directories is if the directory is generated, right? That should be comparatively rare, and the generation script can always clean out the directory first before it reruns. I'm not sure we need any clobber or reverse cleans for this.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46dc99a535e2fe9935ac5338b4e28e134dd558c3

commit 46dc99a535e2fe9935ac5338b4e28e134dd558c3
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Aug 08 14:32:00 2018

Remove directory dependency to pyproto/google

Directory dependency to pyproto/google seems not necessary for
unit_tests and interactive_ui_tests

Bug: 869348
Change-Id: I6380754639a8e4713bf8a380f2050a0422987dd4
Reviewed-on: https://chromium-review.googlesource.com/1167146
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581557}
[modify] https://crrev.com/46dc99a535e2fe9935ac5338b4e28e134dd558c3/chrome/test/BUILD.gn

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a78cf8ea88d186c78ddbf087f6d2d9dbbc6d1bd9

commit a78cf8ea88d186c78ddbf087f6d2d9dbbc6d1bd9
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Aug 08 16:29:14 2018

Remove data dependency to pyproto dir

Specify neccessary python protos instead.

This is for deterministic tarball sent to swarming for browser_test
execution.

Bug: 869348
Change-Id: I99e2eedb6e38c89d73bad522c86de5c167c05828
Reviewed-on: https://chromium-review.googlesource.com/1166764
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581586}
[modify] https://crrev.com/a78cf8ea88d186c78ddbf087f6d2d9dbbc6d1bd9/chrome/test/BUILD.gn

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f170144cafe3ef60b54027f0dc4856a5316be8c5

commit f170144cafe3ef60b54027f0dc4856a5316be8c5
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Aug 08 22:06:23 2018

Remove directory dependency to pyproto

Specify neccessary python protos instead.

This is for deterministic tarballs sent to swarming for
sync_integration_tests execution.

Bug: 869348
Change-Id: Ifc873081056b7ee19f13a5d819829a05e3179d45
Reviewed-on: https://chromium-review.googlesource.com/1167343
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581703}
[modify] https://crrev.com/f170144cafe3ef60b54027f0dc4856a5316be8c5/chrome/test/BUILD.gn
[modify] https://crrev.com/f170144cafe3ef60b54027f0dc4856a5316be8c5/components/sync/protocol/BUILD.gn
[modify] https://crrev.com/f170144cafe3ef60b54027f0dc4856a5316be8c5/components/sync/protocol/protocol_sources.gni

Deduplication for browser_tests happened.
37 out of 7009 in 24 hours.

https://chromium-swarm.appspot.com/tasklist?c=name&c=state&c=created_ts&c=duration&c=pending_time&c=pool&c=bot&et=1533767460000&f=state%3ADEDUPED&f=name%3Abrowser_tests&f=buildername%3Alinux_chromium_rel_ng&l=50&s=created_ts%3Adesc&st=1533680940000

Thanks!

Interesting. Is that with all the sources of non-determinism fixed, or are there other culprits?

#45,
For browser_tests on linux_chromium_rel_ng CQ builder, all of known non-determinism is fixed.
But there may be some directory dependency having temporal non-deterministic build output.

Have you repeated the smoke test (i.e. send a few try jobs for a test cl with only a comment change, check if they get deduped)?

#47
Doing here.
https://chromium-review.googlesource.com/c/chromium/src/+/1160840/8

Note: Even if we only changes comment, that may change binary due to line number from LOG(...).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ff839daceebd9e74e0a9c4755b51615d0a8a550

commit 7ff839daceebd9e74e0a9c4755b51615d0a8a550
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Thu Aug 09 03:15:36 2018

Narrow data dependency to pywebsocket

If data dependency contains entire pytwebsocket directory, input for test
execution on swarming contains .git directory.
https://bugs.chromium.org/p/chromium/issues/detail?id=869348#c29

This makes it difficult to do test execution cache sharing between different
builds.

This CL changed only depend on src/mod_pywebsocket instead.

R=yhirano@chromium.org, jam@chromium.org

Bug: 869348
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I435874f73063ec6cc14b2285806b0e237cd2a940
Reviewed-on: https://chromium-review.googlesource.com/1167009
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581774}
[modify] https://crrev.com/7ff839daceebd9e74e0a9c4755b51615d0a8a550/BUILD.gn
[modify] https://crrev.com/7ff839daceebd9e74e0a9c4755b51615d0a8a550/chrome/test/BUILD.gn
[modify] https://crrev.com/7ff839daceebd9e74e0a9c4755b51615d0a8a550/components/sync/BUILD.gn
[modify] https://crrev.com/7ff839daceebd9e74e0a9c4755b51615d0a8a550/content/test/BUILD.gn
[modify] https://crrev.com/7ff839daceebd9e74e0a9c4755b51615d0a8a550/extensions/BUILD.gn
[modify] https://crrev.com/7ff839daceebd9e74e0a9c4755b51615d0a8a550/net/BUILD.gn
[modify] https://crrev.com/7ff839daceebd9e74e0a9c4755b51615d0a8a550/services/network/BUILD.gn
[modify] https://crrev.com/7ff839daceebd9e74e0a9c4755b51615d0a8a550/third_party/pywebsocket/BUILD.gn
[modify] https://crrev.com/7ff839daceebd9e74e0a9c4755b51615d0a8a550/third_party/pywebsocket/README.chromium

isolate input of browser_test in below try 
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_rel_ng/160142
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_rel_ng/160143

I see the difference in nacl_helper_bootstrap
$ diff -r a53 514
Only in 514: 514c44ef9642d33050ec323f1a848814d08fd266.tar
Only in a53: a5384742f6b42f70a79538ad4384513486befa5b.tar
Binary files a53/out/Release/nacl_helper_bootstrap and 514/out/Release/nacl_helper_bootstrap differ

and snapshot_blob.bin, v8_context_snapshot.bin

These are not yet deterministic in linux_chromium_rel_ng builder.

We appear to be having a positive effect on deduplicated tasks.

https://chromium-swarm.appspot.com/tasklist?c=name&c=state&c=created_ts&c=duration&c=pending_time&c=pool&c=bot&et=1535499840000&f=buildername-tag%3Alinux_chromium_rel_ng&l=50&s=created_ts%3Adesc&st=1535327040000

In the last 2 days, we saw 26k deduplicated out of 150k (17%) up from 3% in c#5. 

#51, I think that is nice results.
Currently most deduplication seems to happen on small test.

For time consuming test, I'd like to know which file in isolated inputs tends to prevent deduplication.

maruel@, 

Does isolateserver have some api to get file list of isolate input and its content?
e.g. from https://isolateserver.appspot.com/browse?namespace=default-gzip&hash=da824eec0652224e3bc5d008ff339cae5d433f6f

Also do we have a recommended way to get swarming task list like json replied from below URL in browser?
https://chromium-swarm.appspot.com/_ah/api/swarming/v1/tasks/list?limit=50&start=1535466720&tags=buildername%3Alinux_chromium_rel_ng&tags=name%3Abrowser_tests

Not the server itself; you use the client to fetch the tree of isolated files. There's no command right now to only fetch the isolated files without the actual content, but this is definitely doable.


The equivalent command for the second question is:
./swarming.py query --limit 50 -S chromium-swarm.appspot.com 'tasks/list?start=1535466720&tags=buildername%3Alinux_chromium_rel_ng&tags=name%3Abrowser_tests'

You can use --json flag to dump it into a file or you can pipe into another command, as you prefer.

#53, Thank you, I'll make investigation tool.

Something else we may want to consider: If a patch adds whitespace before a macro that uses __LINE__ [e.g. LOG()], then that will cause a difference in the resulting binary, even though functionally the binary hasn't changed. 

I wrote attached code taking hash list of isolate inputs.

In the last 200 browser_tests tasks, 28 tasks can be deduplicated if isolate input contains only browser_tests.

Deleted: isolateanalyze.go 2.8 KB

isolateanalyze.go 2.8 KB View Download

Deleted: isolate.csv.tar.gz 5.8 MB

isolate.csv.tar.gz 5.8 MB Download

What do you mean by "if isolate input contains only browser_tests" ?

#57

In other words, if we can assume that all isolated input files are identical other than browser_tests, 28 tasks can be deduplicated out of 200 browser_tests tasks.

The progression mentioned in comment 51 is likely due to https://chromium-review.googlesource.com/c/chromium/src/+/1213525

The dedup rate on Windows has increased from 1% to 18.86%.

Before: https://chromium-swarm.appspot.com/tasklist?c=name&c=state&c=created_ts&c=duration&c=pending_time&c=pool&c=bot&et=1535499840000&f=buildername-tag%3Awin7_chromium_rel_ng&l=50&s=created_ts%3Adesc&st=1535327040000

After: https://chromium-swarm.appspot.com/tasklist?c=name&c=state&c=created_ts&c=duration&c=pending_time&c=pool&c=bot&et=1539128640000&f=buildername-tag%3Awin7_chromium_rel_ng&l=50&s=created_ts%3Adesc&st=1538955840000

This is likely due to https://reviews.llvm.org/rLLD342334 . exes are still dependent on the abs path to the build directory (https://bugs.chromium.org/p/chromium/issues/detail?id=330260#c62 -- but I have patches in flight to fix this for symbol_level=1 at least, see comment 66 over there), but I suppose all chrome build bots currently use the same build dir.

So Windows is now at parity with linux for test deduping; increasing the rate more means making sure that all browser_tests isolate inputs are deterministic.

Woooh! That's awesome. :)

Wow, really great work!

Only 3 hours of data, but now that v8_context_snapshot.bin is deterministic the Win dedup rate is at 31%: https://chromium-swarm.appspot.com/tasklist?c=name&c=state&c=created_ts&c=duration&c=pending_time&c=pool&c=bot&et=1540255440000&f=buildername-tag%3Awin7_chromium_rel_ng&l=50&s=created_ts%3Adesc&st=1540248240000 Just that run has dedup'd net_unittests, media_unittests, shell_dialogs_unittests, services_unittests, webkit_unit_tests, headless_browsertests, extensions_browsertests, device_unittests, content_unittests, content_browsertests, base_unittests, angle_unittests -- most medium-size binaries.

browser_tests can't possibly be dedup'd yet because of  issue 870611 .

webkit_layout_tests seems to never be dedup'd, I'm guessing the "isolate webkit_layout_tests_exparchive" step does something nondeterministic we don't check for yet.


The linux_chromium_rel_ng bot seems to have a slightly lower hit rate at 27.6% (https://chromium-swarm.appspot.com/tasklist?c=name&c=state&c=created_ts&c=duration&c=pending_time&c=pool&c=bot&et=1540255440000&f=buildername-tag%3Alinux_chromium_rel_ng&l=50&s=created_ts%3Adesc&st=1540248240000), but its hit rate has also increased. Issue 804869 suggests that incremental builds might be dependent somehow, which might keep this number down.



use_dummy_lastchange so far seems to have no positive effect. Once we figured out the incremental build thing on linux and the remoting zip thing, we can probably remove it again if the linux bot still has a lower hit rate than windows.

Summary:
- audit webkit_layout_tests isolating step for determinism
- fix remoting zip thing to make browser_tests.isolate deterministic
- figure out apparent nondeterminism on incremental builds
- re-measure, possibly remove use_dummy_lastchange

Here's a theory for why hit rate went up on Linux too: the V8 context snapshot used to contain a few pointers. On Windows, the generating binary was built with ASLR so the pointers were different on EAC run, but on Linux the generator used to not use ASLR so it has deterministic output. So why the improvement? Not writing pointers is _more_ deterministic. Picture something inconsequential changing in blink. That's likely enough to shift symbols in the snapshot generator around (the snapshot includes blink stuff) and to change the pointers, but in a way that has no functional effect. Now the generator wires identical snapshots instead, which means more tests can be cached.

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/ce0a8f3c8fa7025b5836d80745068484a457cb4e

commit ce0a8f3c8fa7025b5836d80745068484a457cb4e
Author: Nico Weber <thakis@chromium.org>
Date: Mon Dec 03 15:48:33 2018

List vulkan_core.h as input of generate_vulkan_layers_json.py.

Since this was missing, the layer json files didn't get regenerated on
vulkan rolls, leading to stale generated json files, which in turn led
to incremental builds having different files in the swarming isolate
than full builds.

To make this type of bug harder to introduce, rewrite
generate_vulkan_layers_json.py a bit:

- pass in path to vulkan_core.h as an argument
- also pass in the input .json / .json.in files as arguments,
  so that the script re-runs if a .json or .json.in input is added
  or removed, and in the script verify that the passed-in list matches
  the glob() the script did previously (this verifies that the sources
  list in the .gn file is up-to-date with the state on disk)
- generate outputs list in gn from sources list, to make sure they're
  in sync
- use an expicit --icd flag instead of doing `'icd' in path`
- fail when failing to extract vk_version instead of silently using a
  default
- some minor python style fixes

Bug:  chromium:910699 ,chromium:869348
Change-Id: I1e598f4566697a7f1ef56b040e52d0717f7ad075
Reviewed-on: https://chromium-review.googlesource.com/c/1358631
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>

[modify] https://crrev.com/ce0a8f3c8fa7025b5836d80745068484a457cb4e/scripts/generate_vulkan_layers_json.py
[modify] https://crrev.com/ce0a8f3c8fa7025b5836d80745068484a457cb4e/third_party/vulkan-tools/BUILD.gn
[modify] https://crrev.com/ce0a8f3c8fa7025b5836d80745068484a457cb4e/third_party/vulkan-validation-layers/BUILD.gn

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b45b2eaa206f2d443e7a58703c4bcbcba837a876

commit b45b2eaa206f2d443e7a58703c4bcbcba837a876
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Mon Dec 03 18:39:55 2018

Roll src/third_party/angle 317a9ebdb019..c10a023a66f2 (6 commits)

https://chromium.googlesource.com/angle/angle.git/+log/317a9ebdb019..c10a023a66f2


git log 317a9ebdb019..c10a023a66f2 --date=short --no-merges --format='%ad %ae %s'
2018-12-03 jmadill@chromium.org Fix fuchsia build of libfeature_support.
2018-12-03 thakis@chromium.org List vulkan_core.h as input of generate_vulkan_layers_json.py.
2018-12-03 Tom.Tan@microsoft.com Don't copy d3dcompiler_47.dll when build target is Windows ARM64
2018-12-03 jmadill@chromium.org Refactor test shader style.
2018-11-30 syoussefi@chromium.org Vulkan: Add dynamic index buffers to graph
2018-11-30 ianelliott@google.com Version-2 API of the A4A opt-in/out (a.k.a. feature-support utilities)


Created with:
  gclient setdep -r src/third_party/angle@c10a023a66f2

The AutoRoll server is located here: https://autoroll.skia.org/r/angle-chromium-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:910699 ,chromium:869348,chromium:893460,chromium:b/113346561
TBR=ynovikov@chromium.org

Change-Id: I29ee936a536269990c413cdd50ad7d46601914a6
Reviewed-on: https://chromium-review.googlesource.com/c/1358758
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#613170}
[modify] https://crrev.com/b45b2eaa206f2d443e7a58703c4bcbcba837a876/DEPS

I had run an experiment at https://chromium-review.googlesource.com/c/chromium/src/+/1296616 where I sent the same CL to linux_chromium_rel_ng several times. I then downloaded the browser_test isolateds produced by the jobs and compared them. api_version being different in VkLayer_foo.json was the main difference I remember. The new Deterministic Linux build dir setup (issue 899438) made the bot find how that could happen, and the thakis commit listed in the roll commit in comment 70 fixed that issue. I will repeat the experiment now.

Another short data point: Here are two builds on Win 7 Tests x64 (1) from last night (a Sunday) when the tree wasn't very busy:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win%207%20Tests%20x64%20%281%29/45948
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win%207%20Tests%20x64%20%281%29/45949

Both builds just contain a single "Roll src-internal ..." change, which shouldn't affect build outputs. So in theory the second roll should have all test steps deduped. It has 76 test steps deduped, which is very good, but it's not all. One thing that I notice is that no sharded test binaries are deduped -- maybe deduping doesn't yet work with sharded runs? Or maybe that's just a coincidence.

Oh wait, that might just be due to LASTCHANGE since it's at different commits (cf use_dummy_lastchange which isn't set on most bots, etc)

Here are the 35 out of 111 tests we didn't dedupe on that run:
app_shell_unittests
blink_heap_unittests
browser_tests
chrome_cleaner_unittests
chrome_elf_import_unittests
chrome_elf_unittests
chromedriver_py_tests
chromedriver_unittests
components_browsertests
components_perftests
components_unittests
content_browsertests
content_shell_crash_test
content_unittests
elevation_service_unittests
extensions_browsertests
extensions_unittests
gcp_unittests
headless_browsertests
installer_util_unittests
interactive_ui_tests
perfetto_content_browsertests
sync_integration_tests
telemetry_gpu_unittests
telemetry_perf_unittests
telemetry_unittests
unit_tests
views_perftests
webkit_unit_tests

(these are dupes of above tests:)
network_service_browser_tests
network_service_components_browsertests
network_service_content_browsertests
network_service_extensions_browsertests
network_service_interactive_ui_tests
webui_polymer2_browser_tests
webui_polymer2_interactive_ui_tests

I repeated my experiment of sending 4 try jobs. browser_tests didn't get deduped even though all 4 jobs were at the same rev. I downloaded 2 of the isolates like so:

Nicos-MacBook-Pro:src thakis$ python tools/swarming_client/isolateserver.py download -I https://isolateserver.appspot.com --namespace default-gzip -s 292c88da922191ee323efc8851676a8dfb3c48bc --target brote1

Nicos-MacBook-Pro:src thakis$ python tools/swarming_client/isolateserver.py download -I https://isolateserver.appspot.com --namespace default-gzip -s b2b49fcd5f68318529253374b034e85f728fea23 --target brote2


I then ran `opendiff brote1 brote2`. FileMerge showed them as equal until I went to Preferences->Filters and cleaned the "Files to ignore" list at the bottom.


Then it showed that these files are different:

chrome/test/data/xr/webvr_{info,samples}/.git/{index,logs/}
third_party/pyftplib/src/.git/{index,logs/}

So somehow we manage to ship (at least) three .git directories in the isolate :-/


But it also looks like we're pretty close to having identical isolates for browser_tests.

Make that pyftpdlib with a d.

Why are the .git contents different? For the logs/ folder at least, it's because git stores timestamps on when it updated stuff.

The easiest fix is probably to add .git to isolate.py's blacklist.

https://chromium.googlesource.com/chromium/tools/build/+/fe3beb864f4d53894d554ddfbe38fc08c9033031 added:

	100	  def _blacklist_args_for_isolate(self):
101	    # Files that match these regexes should never be included in the isolate.
102	    blacklist = ['*.pyc', '*.swp', '.git']
103	    args = []
104	    for el in blacklist:
105	      args.extend(('--blacklist', '"' + el + '"'))
106	    return args

I suppose '.git' only matches .git at the toplevel and not foo/bar/.git?

The default blacklist is (in the py isolate.py, don't know about the go one):

	DEFAULT_BLACKLIST = (
85	  # Temporary vim or python files.
86	  r'^.+\.(?:pyc|swp)$',
87	  # .git or .svn directory.
88	  r'^(?:.+' + re.escape(os.path.sep) + r'|)\.(?:git|svn)$',
89	)

See  issue 872245  for discussions about the blacklist behavior.

Huh, --blacklist wants a regex from what I can tell, but we pass

https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8927223062922368768/+/steps/isolate_tests/0/stdout

 '--blacklist',
 '"*.pyc"',
 '--blacklist',
 '"*.swp"',
 '--blacklist',
 '".git"',

Which I think means "a quote, repeated 0 or more times, followed by any chraacter, followed by p, y, c, another quote".

So I think after erikchen's patch, nothing's getting blacklisted anymore.

On my current attempt, I triggered 4 identical try jobs on a whitespace CL. None of the browser_tests was deduped, but on all 4 runs the first shard had the same isolated hash (!) (didn't look at the other shards, probably true for them too):

https://chromium-swarm.appspot.com/task?id=41c6fee1ac3a5110&refresh=10&show_raw=1
https://chromium-swarm.appspot.com/task?id=41c6fbb322e04510&refresh=10&show_raw=1
https://chromium-swarm.appspot.com/task?id=41c6fbb5376e1a10&refresh=10&show_raw=1
https://chromium-swarm.appspot.com/task?id=41c6fd1460e88510&refresh=10&show_raw=1


Talking to maruel, it sounds like swarming can writes into the dedupe store only after a test has completed and pending test runs aren't yet deduped. Issue 915342 tracks implementing deduping of in-progress tasks.