New issue
Advanced search Search tips

Issue 672182 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

ExecLinkWrapper buffers all output, fails if it gets too large

Project Member Reported by brucedaw...@chromium.org, Dec 7 2016

Issue description

While doing verbose linking of a full LTCG build of chrome.dll, using these settings:

is_chrome_branded = true
is_component_build = false
is_debug = false
target_cpu = "x86"
is_official_build = true
full_wpo_on_official = true

the linker produces roughly 1.1 GB of output. When ExecLinkWrapper in tool_wrapper.py processes this output it uses communicate() which grabs it as one huge chunk. It then splits it into lines which doubles the memory footprint to a minimum of 2.2 GB. Since the depot_tools version of python is 32-bit (and is not large-address-aware) this necessarily causes an out-of-memory failure in the Python script, and all of the output (which takes an hour to generate) is lost.

We either need to switch to 64-bit Python or we need to process the output one line at a time. Changing Python to be large-address-aware might work but does not give a lot of breathing room.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 8 2016

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

commit 5d0d1b0bf01acb3ebe3a5ef27ebf2f3180ab1d1d
Author: brucedawson <brucedawson@chromium.org>
Date: Thu Dec 08 01:47:59 2016

Change ExecLinkWrapper to not buffer all tool output

/verbose linking of chrome.dll creates over one GB of output. This
causes ExecLinkWrapper to consume over two GB of memory which leads to
an OOM failure in the 32-bit depot_tools python, and the loss of all
of the valuable output. This change modifies ExecLinkWrapper to
process the output one line at a time, thus avoiding the OOM.

I've tested that this handles the 1.1 GB of output which the previous
version of this function failed on and I've visually confirmed that the
output looks the same - no extraneous blank lines, for instance, when
displaying warnings, errors, or 1.9 million lines of verbose output.

I also verified that the script stays idle when waiting for output -
blocking on .readline().

BUG= 672182 

Review-Url: https://codereview.chromium.org/2558153002
Cr-Commit-Position: refs/heads/master@{#437126}

[modify] https://crrev.com/5d0d1b0bf01acb3ebe3a5ef27ebf2f3180ab1d1d/build/toolchain/win/tool_wrapper.py

Status: Fixed (was: Started)
This is working nicely - I can grab 1.1 GB verbose outputs without problems now.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 9 2016

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

commit 8e0b754f6372a0d5fc6d1c1df1597dfb9ac877ed
Author: scottmg <scottmg@chromium.org>
Date: Fri Dec 09 17:20:18 2016

Revert of Change ExecLinkWrapper to not buffer all tool output (patchset #2 id:20001 of https://codereview.chromium.org/2558153002/ )

Reason for revert:
Reverting under suspicion for  crbug.com/672841 .

Original issue's description:
> Change ExecLinkWrapper to not buffer all tool output
>
> /verbose linking of chrome.dll creates over one GB of output. This
> causes ExecLinkWrapper to consume over two GB of memory which leads to
> an OOM failure in the 32-bit depot_tools python, and the loss of all
> of the valuable output. This change modifies ExecLinkWrapper to
> process the output one line at a time, thus avoiding the OOM.
>
> I've tested that this handles the 1.1 GB of output which the previous
> version of this function failed on and I've visually confirmed that the
> output looks the same - no extraneous blank lines, for instance, when
> displaying warnings, errors, or 1.9 million lines of verbose output.
>
> I also verified that the script stays idle when waiting for output -
> blocking on .readline().
>
> BUG= 672182 
>
> Committed: https://crrev.com/5d0d1b0bf01acb3ebe3a5ef27ebf2f3180ab1d1d
> Cr-Commit-Position: refs/heads/master@{#437126}

TBR=brucedawson@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 672182 

Review-Url: https://codereview.chromium.org/2564893002
Cr-Commit-Position: refs/heads/master@{#437572}

[modify] https://crrev.com/8e0b754f6372a0d5fc6d1c1df1597dfb9ac877ed/build/toolchain/win/tool_wrapper.py

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 9 2016

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

commit 81dfeb7be03b9b3b7311ede93908fd8159761446
Author: brucedawson <brucedawson@chromium.org>
Date: Fri Dec 09 23:39:05 2016

Change ExecLinkWrapper to not buffer all tool output

/verbose linking of chrome.dll creates over one GB of output. This
causes ExecLinkWrapper to consume over two GB of memory which leads to
an OOM failure in the 32-bit depot_tools python, and the loss of all
of the valuable output. This change modifies ExecLinkWrapper to
process the output one line at a time, thus avoiding the OOM.

I've tested that this handles the 1.1 GB of output which the previous
version of this function failed on and I've visually confirmed that the
output looks the same - no extraneous blank lines, for instance, when
displaying warnings, errors, or 1.9 million lines of verbose output.

I also verified that the script stays idle when waiting for output -
blocking on .readline().

This fixes the previous attempt which omitted the vital link.wait() call
which meant that error codes were not propagated.

R=scottmg@chromium.org
BUG= 672182 , 672841 

Review-Url: https://codereview.chromium.org/2568563002
Cr-Commit-Position: refs/heads/master@{#437696}

[modify] https://crrev.com/81dfeb7be03b9b3b7311ede93908fd8159761446/build/toolchain/win/tool_wrapper.py

Sign in to add a comment