Re: Review Request 28272: Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/#review62607 --- Ship it! Ship It! - David McLaughlin On Nov. 21, 2014, 1:29 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/ --- (Updated Nov. 21, 2014, 1:29 a.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-930 https://issues.apache.org/jira/browse/AURORA-930 Repository: aurora Description --- Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task. Diffs - src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java 7b46740a5beb4b5bde4b5d41bea9573195e047b9 Diff: https://reviews.apache.org/r/28272/diff/ Testing --- ./gradlew -Pq build Suggestions on how to better test this are welcome. Thanks, Kevin Sweeney
Re: Review Request 28272: Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/#review62391 --- Ship it! LGTM once more history is included in comments. src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java https://reviews.apache.org/r/28272/#comment104442 // Copy all fields in AssignedTask except the TASK field. - Bill Farner On Nov. 20, 2014, 1:08 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/ --- (Updated Nov. 20, 2014, 1:08 a.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-930 https://issues.apache.org/jira/browse/AURORA-930 Repository: aurora Description --- Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task. Diffs - src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java 7b46740a5beb4b5bde4b5d41bea9573195e047b9 Diff: https://reviews.apache.org/r/28272/diff/ Testing --- ./gradlew -Pq build Suggestions on how to better test this are welcome. Thanks, Kevin Sweeney
Re: Review Request 28272: Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/ --- (Updated Nov. 20, 2014, 5:29 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Changes --- Add comments. Bugs: AURORA-930 https://issues.apache.org/jira/browse/AURORA-930 Repository: aurora Description --- Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java 7b46740a5beb4b5bde4b5d41bea9573195e047b9 Diff: https://reviews.apache.org/r/28272/diff/ Testing --- ./gradlew -Pq build Suggestions on how to better test this are welcome. Thanks, Kevin Sweeney
Re: Review Request 28272: Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task.
On Nov. 19, 2014, 6:01 p.m., David McLaughlin wrote: src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java, line 93 https://reviews.apache.org/r/28272/diff/1/?file=770796#file770796line93 Can you add a comment explaining why we're doing this? Done. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/#review62317 --- On Nov. 20, 2014, 5:29 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/ --- (Updated Nov. 20, 2014, 5:29 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-930 https://issues.apache.org/jira/browse/AURORA-930 Repository: aurora Description --- Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task. Diffs - src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java 7b46740a5beb4b5bde4b5d41bea9573195e047b9 Diff: https://reviews.apache.org/r/28272/diff/ Testing --- ./gradlew -Pq build Suggestions on how to better test this are welcome. Thanks, Kevin Sweeney
Re: Review Request 28272: Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/#review62500 --- Master (ada97bd) is green with this patch. ./build-support/jenkins/build.sh However, it appears that it might lack test coverage. I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Nov. 21, 2014, 1:29 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/ --- (Updated Nov. 21, 2014, 1:29 a.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-930 https://issues.apache.org/jira/browse/AURORA-930 Repository: aurora Description --- Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task. Diffs - src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java 7b46740a5beb4b5bde4b5d41bea9573195e047b9 Diff: https://reviews.apache.org/r/28272/diff/ Testing --- ./gradlew -Pq build Suggestions on how to better test this are welcome. Thanks, Kevin Sweeney
Review Request 28272: Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/ --- Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-930 https://issues.apache.org/jira/browse/AURORA-930 Repository: aurora Description --- Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task. Diffs - src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java 7b46740a5beb4b5bde4b5d41bea9573195e047b9 Diff: https://reviews.apache.org/r/28272/diff/ Testing --- ./gradlew -Pq build Suggestions on how to better test this are welcome. Thanks, Kevin Sweeney
Re: Review Request 28272: Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/#review62312 --- Master (065a3c5) is red with this patch. ./build-support/jenkins/build.sh Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/pants.venv/lib/python2.7/site-packages/twitter.common.util-0.3.1-py2.7-nspkg.pth Found existing installation: setuptools 3.6 Uninstalling setuptools: Successfully uninstalled setuptools Successfully installed Markdown Pygments ansicolors cov-core coverage lockfile pantsbuild.pants pex psutil py pystache pytest pytest-cov python-daemon requests twitter.common.collections twitter.common.config twitter.common.confluence twitter.common.contextutil twitter.common.decorators twitter.common.dirutil twitter.common.lang twitter.common.log twitter.common.options twitter.common.process twitter.common.string twitter.common.threading twitter.common.util setuptools Cleaning up... Build operating on top level addresses: set([BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/BUILD, all)]) Problem executing PythonBuilder for targets OrderedSet([PythonTests(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/BUILD, host_maintenance)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/client/BUILD, api)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/client/api/BUILD, api)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/client/api/BUILD, restarter)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/client/api/BUILD, instance_watcher)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/client/api/BUILD, scheduler_client)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/au rora/common/auth/BUILD, auth)), PythonThriftLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/thrift/org/apache/aurora/gen/BUILD, py-thrift)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/common/BUILD, cluster)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/common/BUILD, transport)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/client/api/BUILD, scheduler_mux)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/client/api/BUILD, error_handling_thread)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/client/api/BUILD, task_util)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/client/ BUILD, base)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/common/BUILD, http_signaler)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/client/api/BUILD, updater_util)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/client/api/BUILD, sla)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/common/BUILD, common)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/common/BUILD, aurora_job_key)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/common/BUILD, cluster_option)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/common/BUILD, clusters)), PythonLibrary(B uildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/common/BUILD, shellify)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/thrift/org/apache/aurora/gen/BUILD, py-thrift-packaged)), PythonThriftLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/thrift/org/apache/aurora/gen/BUILD, py-thrift-test)), PythonThriftLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/thrift/org/apache/aurora/gen/BUILD, py-thrift-storage)), PythonLibrary(BuildFileAddress(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/client/api/BUILD, updater)),
Re: Review Request 28272: Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/#review62316 --- Master (065a3c5) is green with this patch. ./build-support/jenkins/build.sh However, it appears that it might lack test coverage. I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Nov. 20, 2014, 1:08 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/ --- (Updated Nov. 20, 2014, 1:08 a.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-930 https://issues.apache.org/jira/browse/AURORA-930 Repository: aurora Description --- Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task. Diffs - src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java 7b46740a5beb4b5bde4b5d41bea9573195e047b9 Diff: https://reviews.apache.org/r/28272/diff/ Testing --- ./gradlew -Pq build Suggestions on how to better test this are welcome. Thanks, Kevin Sweeney
Re: Review Request 28272: Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/#review62317 --- In addition to having a comment in code, can you add some info to this review or the JIRA ticket about how you came to the conclusion that this will benefit? I'm assuming this is just designed to reduce object allocations and GC churn? What sort of improvement in terms of % of objects created during a snapshot should we expect? src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java https://reviews.apache.org/r/28272/#comment104342 Can you add a comment explaining why we're doing this? - David McLaughlin On Nov. 20, 2014, 1:08 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28272/ --- (Updated Nov. 20, 2014, 1:08 a.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-930 https://issues.apache.org/jira/browse/AURORA-930 Repository: aurora Description --- Avoid creating garbage copies of Snapshot#tasks and AssignedTask#task. Diffs - src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java 7b46740a5beb4b5bde4b5d41bea9573195e047b9 Diff: https://reviews.apache.org/r/28272/diff/ Testing --- ./gradlew -Pq build Suggestions on how to better test this are welcome. Thanks, Kevin Sweeney
