Update PythonZipper action to use CommandLineItem.CapturingMapFn#15472
Merged
ckolli5 merged 1 commit intobazelbuild:release-5.2.0from May 19, 2022
Merged
Conversation
Contributor
Author
|
I have no clue what that windows failure is about. The error message is not meaningful to me. |
|
@philsc Could you please rebase your branch with the latest release-5.2.0 branch? Thanks! |
### Summary This PR attempts to address bazelbuild#14890. This updates the PythonZipper action to use a `CommandLineItem.CapturingMapFn` to defer expanding arguments until after analysis. I used `CapturingMapFn`, as it seemed like the only way to go about implementing the arg filtering that is done by the current code to exclude the `executable` and `zipFile` artifacts. ### Test Plan Use the example from bazelbuild#14890 of looking at memory usage in tensorflow before/after this patch is applied on top of version `5.0.0` Initial Setup ``` git clone [email protected]:tensorflow/tensorflow.git cd tensorflow python3 -m venv venv source venv/bin/activate pip install numpy ``` View memory usage at 5.0.0: ```bash # STARTUP_FLAGS from https://bazel.build/rules/performance#enabling-memory-tracking $ bazel ${STARTUP_FLAGS[@]} build --nobuild //tensorflow/python/... $ bazel ${STARTUP_FLAGS[@]} dump --rules | egrep '(RULE|py_)' RULE COUNT ACTIONS BYTES EACH py_test 1,714 15,390 413,338,632 241,154 py_library 1,102 0 2,097,152 1,903 py_binary 33 198 8,914,840 270,146 py_runtime 6 0 0 0 py_runtime_pair 3 0 0 0 _concat_flatbuffer_py_srcs 2 2 0 0 $ bazel ${STARTUP_FLAGS[@]} info used-heap-size-after-gc 635MB ``` View memory usage at 5.0.0 with this patch applied: ```bash $ ~/code/bazel/bazel-bin/src/bazel ${STARTUP_FLAGS[@]} dump --rules | egrep '(RULE|py_)' RULE COUNT ACTIONS BYTES EACH py_test 1,714 15,390 65,323,312 38,111 py_library 1,102 0 2,359,576 2,141 py_binary 33 198 524,400 15,890 py_runtime 6 0 0 0 py_runtime_pair 3 0 0 0 _concat_flatbuffer_py_srcs 2 2 0 0 $ ~/code/bazel/bazel-bin/src/bazel ${STARTUP_FLAGS[@]} info used-heap-size-after-gc 283MB ``` Ensure that the generated actions have not changed: ```bash $ bazel aquery --build_python_zip 'mnemonic("PythonZipper", //tensorflow/python/...)' > /tmp/bazel_5_out $ ~/code/bazel/bazel-bin/src/bazel aquery --build_python_zip 'mnemonic("PythonZipper", //tensorflow/python/...)' > /tmp/bazel_5_patched_out $ diff /tmp/bazel_5_out /tmp/bazel_5_patched_out # note: no diff output ``` Closes bazelbuild#15037. PiperOrigin-RevId: 441257695 (cherry picked from commit 9610ae8)
d74cdd8 to
4ccbaba
Compare
Contributor
Author
|
Done |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR attempts to address #14890.
This updates the PythonZipper action to use a
CommandLineItem.CapturingMapFnto defer expanding arguments until after analysis. I usedCapturingMapFn, as it seemed like the only way to go about implementing the arg filtering that is done by the current code to exclude theexecutableandzipFileartifacts.Test Plan
Use the example from #14890 of looking at memory usage in tensorflow before/after this patch is applied on top of version
5.0.0Initial Setup
View memory usage at 5.0.0:
View memory usage at 5.0.0 with this patch applied:
Ensure that the generated actions have not changed:
(cherry picked from commit 9610ae8)