New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Do not update timestamp of output file from generator if there is a file with exactly same content

Project Member Reported by tikuta@chromium.org, Feb 8

Issue description

In chromium building, we can reduce the number of incremental tasks when we don't update timestamp of some generated files.

But some generator scripts update timestamp of output files if it has the same content already.

This is tracking bug for work to change such scripts not to update timestamp of output file if it is not necessary.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 8

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

commit 7a8475dbd556242723a6a2e54e51a5dbf5929591
Author: Takuto Ikuta <tikuta@google.com>
Date: Thu Feb 08 15:17:39 2018

Do not update file timestamp if file content is not updated in jinja_template.py

If there is already a file having the same content, we don't need to write to it.
And this reduces the number of build tasks in incremental build.

After building all target with android_n5x_swarming_rel config, I touched jinja_template.py and see the number of incremental build tasks when I rebuild all.
With this CL: 951
Without this CL: 9175

This CL removes at most around 8000 compile tasks in incremental build.

Bug: 810298
Change-Id: Ic7efb33f80c4dc00e3480dcfc2853bdfab612cd6
Reviewed-on: https://chromium-review.googlesource.com/908608
Commit-Queue: Takuto Ikuta <tikuta@google.com>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535384}
[modify] https://crrev.com/7a8475dbd556242723a6a2e54e51a5dbf5929591/build/android/gyp/jinja_template.py

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 8

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

commit 2a4bf9de46bf71de74e22ffc73bcfa681d67cae8
Author: Takuto Ikuta <tikuta@google.com>
Date: Thu Feb 08 21:52:40 2018

Do not update file timestamp if file content is not updated in mojom generators

Timestamp of file should not be updated if there is already the file having the same content.
This CL reduces the number of compile tasks in incremental build.

After building all target with linux_chromium_rel_ng builder config, I touched files in this change and see the number of incremental compile tasks when I rebuild all target.
Without this CL: 25158
With this CL: 7098

This CL reduces at most around 18000 compile tasks in incremental build.

Bug: 810298
Change-Id: Ie7f465dd935b6ae5ee901906a25acea741d42530
Reviewed-on: https://chromium-review.googlesource.com/908268
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@google.com>
Cr-Commit-Position: refs/heads/master@{#535533}
[modify] https://crrev.com/2a4bf9de46bf71de74e22ffc73bcfa681d67cae8/mojo/public/interfaces/bindings/tests/BUILD.gn
[modify] https://crrev.com/2a4bf9de46bf71de74e22ffc73bcfa681d67cae8/mojo/public/tools/bindings/gen_data_files_list.py
[modify] https://crrev.com/2a4bf9de46bf71de74e22ffc73bcfa681d67cae8/mojo/public/tools/bindings/generate_type_mappings.py
[modify] https://crrev.com/2a4bf9de46bf71de74e22ffc73bcfa681d67cae8/mojo/public/tools/bindings/mojom.gni
[modify] https://crrev.com/2a4bf9de46bf71de74e22ffc73bcfa681d67cae8/mojo/public/tools/bindings/mojom_bindings_generator.py
[modify] https://crrev.com/2a4bf9de46bf71de74e22ffc73bcfa681d67cae8/mojo/public/tools/bindings/pylib/mojom/generate/generator.py

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 3

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

commit cca55c33d0728f8429a6ad8ed92a9625171970be
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Tue Jul 03 13:58:31 2018

Use AtomicOutput for java_cpp_enum.py

AtomicOutput does not update file timestamp, and it prevents to invoke additional build steps when the file content is not changed.

Bug: 810298
Change-Id: I4308ffb0c53325f3e4375ef85e7a8a302a8aac7b
Reviewed-on: https://chromium-review.googlesource.com/1124126
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572191}
[modify] https://crrev.com/cca55c33d0728f8429a6ad8ed92a9625171970be/build/android/gyp/java_cpp_enum.py

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 3

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

commit 4daf08d0fbc22bf7773cf3d55786a864784a5b8d
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Tue Jul 03 13:58:45 2018

Use AtomicOutput for aidl.py

AtomicOutput does not update file timestamp, and it prevents to invoke additional build steps when the file content is not changed.

Bug: 810298
Change-Id: I25fd08b2fb4eed98d25bdaf04437e85236391667
Reviewed-on: https://chromium-review.googlesource.com/1124136
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572192}
[modify] https://crrev.com/4daf08d0fbc22bf7773cf3d55786a864784a5b8d/build/android/gyp/aidl.py

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 3

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

commit ce4a404601999f654a6e96aadd523ad21610f64f
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Tue Jul 03 14:04:45 2018

Use AtomicOutput for ZipDir

AtomicOutput does not update file timestamp, and it prevents to invoke additional build steps when the file content is not changed.
This will be affective for some jar related actions. (e.g. mojo/protobuf generator for java)

Bug: 810298
Change-Id: If7b5dd4405214164818b18e49642fb29147667c8
Reviewed-on: https://chromium-review.googlesource.com/1123970
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572194}
[modify] https://crrev.com/ce4a404601999f654a6e96aadd523ad21610f64f/build/android/gyp/util/build_utils.py

Status: Started (was: Untriaged)
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 6

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

commit 990df2af54c5a8add5bf7411a21075167a8171cb
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Fri Jul 06 05:44:45 2018

Use AtomicOutput for faster incremental build on android

Bug: 810298
Change-Id: Ia511b73c8f39d94d3341a811c9a44431c6c43437
Reviewed-on: https://chromium-review.googlesource.com/1127206
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572903}
[modify] https://crrev.com/990df2af54c5a8add5bf7411a21075167a8171cb/build/android/gn/zip.py
[modify] https://crrev.com/990df2af54c5a8add5bf7411a21075167a8171cb/build/android/gyp/create_java_binary_script.py
[modify] https://crrev.com/990df2af54c5a8add5bf7411a21075167a8171cb/build/android/gyp/create_stack_script.py
[modify] https://crrev.com/990df2af54c5a8add5bf7411a21075167a8171cb/build/android/gyp/create_test_runner_script.py
[modify] https://crrev.com/990df2af54c5a8add5bf7411a21075167a8171cb/build/android/gyp/filter_zip.py
[modify] https://crrev.com/990df2af54c5a8add5bf7411a21075167a8171cb/build/android/gyp/gcc_preprocess.py
[modify] https://crrev.com/990df2af54c5a8add5bf7411a21075167a8171cb/build/android/gyp/merge_manifest.py
[modify] https://crrev.com/990df2af54c5a8add5bf7411a21075167a8171cb/build/android/gyp/util/build_utils.py
[modify] https://crrev.com/990df2af54c5a8add5bf7411a21075167a8171cb/build/android/incremental_install/write_installer_json.py

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 7

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

commit ddf8e95d7a6c2dd5eeb13c05a185fd0aba337aab
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Sat Jul 07 00:03:39 2018

Update timestmap of file only when content is changed

This can reduce build steps in incremental build.

I also changed AtomicOutput to avoid some error on windows.

Bug: 810298
Change-Id: I6d076a70b7273bada7e6a94afbd7d805de4b89c2
Reviewed-on: https://chromium-review.googlesource.com/1127510
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573123}
[modify] https://crrev.com/ddf8e95d7a6c2dd5eeb13c05a185fd0aba337aab/build/android/gyp/util/build_utils.py
[modify] https://crrev.com/ddf8e95d7a6c2dd5eeb13c05a185fd0aba337aab/services/catalog/public/tools/generate_manifest.py
[modify] https://crrev.com/ddf8e95d7a6c2dd5eeb13c05a185fd0aba337aab/services/catalog/public/tools/sourcify_manifest.py

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 7

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

commit 53ed5722bc73a243e24880f5b5554da1d3017af6
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Sat Jul 07 03:18:59 2018

Keep timestamp in aar.py and copy_ex.py when file is not changed

This removes unnecessary build steps in incremental build.

Also this mitigates  crbug.com/860251  a bit.

Bug: 810298,  860251 
Change-Id: Ia6aedfea692611a08b1f9a6a2c3963bb69cf32bc
Reviewed-on: https://chromium-review.googlesource.com/1127565
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573145}
[modify] https://crrev.com/53ed5722bc73a243e24880f5b5554da1d3017af6/build/android/gyp/aar.py
[modify] https://crrev.com/53ed5722bc73a243e24880f5b5554da1d3017af6/build/android/gyp/copy_ex.py

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 7

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

commit 688cf20656336fc8d3153e90a2722ca0b4831c23
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Sat Jul 07 03:54:32 2018

Revert "Keep timestamp in aar.py and copy_ex.py when file is not changed"

This reverts commit 53ed5722bc73a243e24880f5b5554da1d3017af6.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 573145 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzUzZWQ1NzIyYmM3M2EyNDNlMjQ4ODBmNWI1NTU0ZGExZDMwMTdhZjYM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium/Android/87881

Sample Failed Step: compile

Original change's description:
> Keep timestamp in aar.py and copy_ex.py when file is not changed
> 
> This removes unnecessary build steps in incremental build.
> 
> Also this mitigates  crbug.com/860251  a bit.
> 
> Bug: 810298,  860251 
> Change-Id: Ia6aedfea692611a08b1f9a6a2c3963bb69cf32bc
> Reviewed-on: https://chromium-review.googlesource.com/1127565
> Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
> Reviewed-by: Eric Stevenson <estevenson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#573145}

Change-Id: Icc99c6e07955df939916de244188330fbc9f3a05
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 810298,  860251 
Reviewed-on: https://chromium-review.googlesource.com/1128479
Cr-Commit-Position: refs/heads/master@{#573146}
[modify] https://crrev.com/688cf20656336fc8d3153e90a2722ca0b4831c23/build/android/gyp/aar.py
[modify] https://crrev.com/688cf20656336fc8d3153e90a2722ca0b4831c23/build/android/gyp/copy_ex.py

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 9

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

commit 4c457d39d71a11ed75e309d9743687c5bb059724
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Mon Jul 09 14:26:29 2018

Update timestmap of file only when content is changed

This can reduce build steps in incremental build.

Bug: 810298
Change-Id: Ie365d765c7bf86a6e8a2bec74c353dcd42a298d1
Reviewed-on: https://chromium-review.googlesource.com/1128675
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573300}
[modify] https://crrev.com/4c457d39d71a11ed75e309d9743687c5bb059724/services/service_manager/public/tools/manifest/manifest_collator.py

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 9

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

commit 18e21ae1b9c0a3857243c69a9436a47935a734ae
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Mon Jul 09 22:15:57 2018

Update timestmap of file only when content is changed

This can reduce build steps in incremental build.

Bug: 810298
Change-Id: I8ef6c3aa908f5d6e2e8c57edf8d29ed80a0d1993
Reviewed-on: https://chromium-review.googlesource.com/1126892
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573483}
[modify] https://crrev.com/18e21ae1b9c0a3857243c69a9436a47935a734ae/content/public/android/generate_child_service.py

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 11

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

commit 6d57d01fad8045040e17708b0256e6c52df2f4ed
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Jul 11 01:25:54 2018

Reland "Keep timestamp in aar.py and copy_ex.py when file is not changed"

This is a reland of 53ed5722bc73a243e24880f5b5554da1d3017af6

But only reland the change for copy_ex.py
Reverting aar.py fixes the build failure in Android builder.

Original change's description:
> Keep timestamp in aar.py and copy_ex.py when file is not changed
>
> This removes unnecessary build steps in incremental build.
>
> Also this mitigates  crbug.com/860251  a bit.
>
> Bug: 810298,  860251 
> Change-Id: Ia6aedfea692611a08b1f9a6a2c3963bb69cf32bc
> Reviewed-on: https://chromium-review.googlesource.com/1127565
> Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
> Reviewed-by: Eric Stevenson <estevenson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#573145}

Bug: 810298,  860251 
Change-Id: Ibd30991d65153553a8c36d1af82552461c85966f
Reviewed-on: https://chromium-review.googlesource.com/1132494
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574029}
[modify] https://crrev.com/6d57d01fad8045040e17708b0256e6c52df2f4ed/build/android/gyp/copy_ex.py

Project Member

Comment 14 by bugdroid1@chromium.org, Jul 12

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

commit 2d186e03f89aa3fa40dc7266cbc33a0310ac3dd8
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Thu Jul 12 16:22:48 2018

Keep timestamp in aar.py when file is not changed

This is a reland of
https://chromium-review.googlesource.com/c/chromium/src/+/1127565
I didn't understand filecmp.dircmp.

Changed to use md5_check.CallAndRecordIfStale instead.
Also let ExtractAll create directory even when it is empty.

Bug: 810298,  860251 
Change-Id: I0d1e2e385ff1a12981473ef422099f638ca174cf
Reviewed-on: https://chromium-review.googlesource.com/1132818
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574589}
[modify] https://crrev.com/2d186e03f89aa3fa40dc7266cbc33a0310ac3dd8/build/android/gyp/aar.py
[modify] https://crrev.com/2d186e03f89aa3fa40dc7266cbc33a0310ac3dd8/build/android/gyp/util/build_utils.py

Sign in to add a comment