Revert of Use --num-thread=10 when dexing Android code (saves 5 seconds)

Revert of Use --num-thread=10 when dexing Android code (saves 5 seconds) (patchset #1 id:1 of https://codereview.chromium.org/1109193002/)

Reason for revert:
This patch makes the creation of dex files non-deterministic -- dex files from different runs of the same build might not be exactly similar.

This is a problem because for 64-bit webview APKs we need to merge a 32-bit APK into the 64-bit APK (to make 32-bit apps work). When we do this we check that the files in the different APks are similar and this check doesn't work if the dex files in the two APKs are not exactly the same.
https://code.google.com/p/chromium/issues/detail?id=483665

Original issue's description:
> Use --num-thread=10 when dexing Android code (saves 5 seconds)
>
> Timing for creating chrome_apk/classes.dex:
>
> No flag:
> real    0m10.767s
> user    0m16.480s
> sys     0m0.444s
>
> --num-threads=4
> real    0m7.450s
> user    0m11.837s
> sys     0m0.606s
>
> --num-threads=10 (or 20, or 80)
> real    0m5.399s
> user    0m9.266s
> sys     0m0.420s
>
> Timings performed on a z620 workstation.
>
> BUG=
>
> Committed: https://crrev.com/0f9ad3a20275b77d228f67c2edd89dbea0e3e9ea
> Cr-Commit-Position: refs/heads/master@{#327358}

TBR=cjhopman@chromium.org,thestig@chromium.org,agrieve@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Committed: https://crrev.com/a9975e3af252dbb1f28b55ed5398731e576b482e
Cr-Commit-Position: refs/heads/master@{#328521}

[gsennton, 2015-05-06 13:08:08]

Created Revert of Use --num-thread=10 when dexing Android code (saves 5 seconds)

[commit-bot: I haz the power, 2015-05-06 13:08:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1129793002/1

[commit-bot: I haz the power, 2015-05-06 13:08:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-06 13:08:20]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[gsennton, 2015-05-06 13:19:48]

gsennton@chromium.org changed reviewers:
+ torne@chromium.org

[Torne, 2015-05-06 13:21:56]

LGTM - sorry folks but for now we need java compilation in official builds to be
deterministic to support the weird hack we do to make working 64-bit webviews.
This could re-land for non-official builds if you want, but we need dev builds
working asap :(

[gsennton, 2015-05-06 13:22:38]

The CQ bit was checked by gsennton@chromium.org

[commit-bot: I haz the power, 2015-05-06 13:23:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1129793002/1

[commit-bot: I haz the power, 2015-05-06 13:24:04]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-05-06 13:24:59]

Patchset 1 (id:??) landed as
https://crrev.com/a9975e3af252dbb1f28b55ed5398731e576b482e
Cr-Commit-Position: refs/heads/master@{#328521}

[agrieve1, 2015-05-07 13:42:50]

On 2015/05/06 13:24:59, I haz the power (commit-bot) wrote:
> Patchset 1 (id:??) landed as
> https://crrev.com/a9975e3af252dbb1f28b55ed5398731e576b482e
> Cr-Commit-Position: refs/heads/master@{#328521}

From your description, it's not entirely clear to me whether turning on
threading when buildtype=Dev is okay. WDYT?

[Torne, 2015-05-07 13:47:07]

On 2015/05/07 13:42:50, agrieve1 wrote:
> On 2015/05/06 13:24:59, I haz the power (commit-bot) wrote:
> > Patchset 1 (id:??) landed as
> > https://crrev.com/a9975e3af252dbb1f28b55ed5398731e576b482e
> > Cr-Commit-Position: refs/heads/master@{#328521}
> 
> From your description, it's not entirely clear to me whether turning on
> threading when buildtype=Dev is okay. WDYT?

Sorry, I meant "official builds for dev channel". It should be fine to enable
this for buildtype!="Official".

[cjhopman, 2015-05-07 17:57:05]

On 2015/05/07 13:47:07, Torne wrote:
> On 2015/05/07 13:42:50, agrieve1 wrote:
> > On 2015/05/06 13:24:59, I haz the power (commit-bot) wrote:
> > > Patchset 1 (id:??) landed as
> > > https://crrev.com/a9975e3af252dbb1f28b55ed5398731e576b482e
> > > Cr-Commit-Position: refs/heads/master@{#328521}
> > 
> > From your description, it's not entirely clear to me whether turning on
> > threading when buildtype=Dev is okay. WDYT?
> 
> Sorry, I meant "official builds for dev channel". It should be fine to enable
> this for buildtype!="Official".

I think that we should probably avoid non-deterministic builds in general.

According to https://code.google.com/p/android/issues/detail?id=79450, the
non-determinism is fixed by
https://android.googlesource.com/platform/dalvik/+/845d9d0eed0f6556e11ee7f720....
Maybe we should just wait until we are using a dx with that change.

[Torne, 2015-05-07 18:02:04]

On 2015/05/07 17:57:05, cjhopman wrote:
> On 2015/05/07 13:47:07, Torne wrote:
> > On 2015/05/07 13:42:50, agrieve1 wrote:
> > > On 2015/05/06 13:24:59, I haz the power (commit-bot) wrote:
> > > > Patchset 1 (id:??) landed as
> > > > https://crrev.com/a9975e3af252dbb1f28b55ed5398731e576b482e
> > > > Cr-Commit-Position: refs/heads/master@{#328521}
> > > 
> > > From your description, it's not entirely clear to me whether turning on
> > > threading when buildtype=Dev is okay. WDYT?
> > 
> > Sorry, I meant "official builds for dev channel". It should be fine to
enable
> > this for buildtype!="Official".
> 
> I think that we should probably avoid non-deterministic builds in general.
> 
> According to https://code.google.com/p/android/issues/detail?id=79450, the
> non-determinism is fixed by
>
https://android.googlesource.com/platform/dalvik/+/845d9d0eed0f6556e11ee7f720....
> Maybe we should just wait until we are using a dx with that change.

Ah, yeah, if it's already fixed to be deterministic in later versions I also
think we should just wait, 5 seconds is nice for edit-compile cycles but not
vital.

[agrieve, 2015-05-07 18:12:20]

sgtm!

On Thu, May 7, 2015 at 2:02 PM, <torne@chromium.org> wrote:

> On 2015/05/07 17:57:05, cjhopman wrote:
>
>> On 2015/05/07 13:47:07, Torne wrote:
>> > On 2015/05/07 13:42:50, agrieve1 wrote:
>> > > On 2015/05/06 13:24:59, I haz the power (commit-bot) wrote:
>> > > > Patchset 1 (id:??) landed as
>> > > > https://crrev.com/a9975e3af252dbb1f28b55ed5398731e576b482e
>> > > > Cr-Commit-Position: refs/heads/master@{#328521}
>> > >
>> > > From your description, it's not entirely clear to me whether turning
>> on
>> > > threading when buildtype=Dev is okay. WDYT?
>> >
>> > Sorry, I meant "official builds for dev channel". It should be fine to
>>
> enable
>
>> > this for buildtype!="Official".
>>
>
>  I think that we should probably avoid non-deterministic builds in general.
>>
>
>  According to https://code.google.com/p/android/issues/detail?id=79450,
>> the
>> non-determinism is fixed by
>>
>
>
>
https://android.googlesource.com/platform/dalvik/+/845d9d0eed0f6556e11ee7f720...
> .
>
>> Maybe we should just wait until we are using a dx with that change.
>>
>
> Ah, yeah, if it's already fixed to be deterministic in later versions I
> also
> think we should just wait, 5 seconds is nice for edit-compile cycles but
> not
> vital.
>
> https://codereview.chromium.org/1129793002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.