[PATCH] D58296: [llvm-objcopy] Make removeSectionReferences batched

Tue Feb 19 15:05:30 PST 2019

rupprecht marked 2 inline comments as done.
rupprecht added a comment.

In D58296#1400258 <https://reviews.llvm.org/D58296#1400258>, @MaskRay wrote:

> The approach looks good to me, but is anyone concerned about the 3MB compressed file?

FWIW, this would be the 11th largest file:

  $ find . -size +1M -print0 | xargs -0 du -sk | grep -v .git | sort -nr
  15752   ./compiler-rt/test/builtins/Unit/udivmodti4_test.c
  8124    ./lldb/unittests/Process/minidump/Inputs/fizzbuzz_wow64.dmp
  8124    ./lldb/packages/Python/lldbsuite/test/functionalities/postmortem/wow64_minidump/fizzbuzz_wow64.dmp
  7416    ./lldb/www/python_reference/lldb-pysrc.html
  5404    ./llvm/test/MC/Disassembler/AMDGPU/gfx9_dasm_all.txt
  5244    ./libcxxabi/test/test_demangle.pass.cpp
  5108    ./llvm/test/MC/Disassembler/AMDGPU/gfx8_dasm_all.txt
  3712    ./llvm/test/tools/sancov/Inputs/test-linux_android_aarch64
  3688    ./llvm/test/MC/AMDGPU/gfx9_asm_all.s
  3508    ./llvm/test/MC/AMDGPU/gfx8_asm_all.s
  3352    ./llvm/test/tools/llvm-objcopy/ELF/Inputs/huge-input.o.gz

As mentioned in a comment though, I plan to pull it from the review.

================
Comment at: llvm/test/tools/llvm-objcopy/ELF/only-section-huge.test:1
+# Regression test that ensures reasonable performance for section removal, i.e.
+# selectively removing almost all sections on a large file takes a couple
----------------
jhenderson wrote:
> How does this test show reasonable performance in a CI run? I don't see anything that checks that the runtime is sane.
It doesn't -- it will just take a very long time, and hopefully buildbots would be configured to run with a timeout and kill the test if it takes too long.

I think I should actually revert the test portion of this change, but I can leave it up in phab until I commit it. Testing timeouts like this doesn't seem very common or well supported with lit. Also, someone on IRC (sorry, I forget who) suggested not writing a test with a timeout, because it could fail if it just happens to run on really slow hardware. Maybe I'll save it and commit it later if there's a better regression testing framework -- there is LNT, but I'm not sure if it's suited to this task.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58296/new/

https://reviews.llvm.org/D58296