----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/#review73233 -----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java <https://reviews.apache.org/r/31107/#comment119501> Why not call this(original, false) to avoid duplicating the code? exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java <https://reviews.apache.org/r/31107/#comment119502> how about final IntLongOpenHashmap fromMetrics = from.longMetrics; final long[] fromValues = fromMetrics.values; for(int i : fromMetrics.keys) { final long value = fromValues[i]; longMetrics.putOrAdd(i, value, value); } This avoids multiple evaluation of the member accesses on every pass of the loop, and multiple bounds checks within the loop body. Similar improvements can be made to the doubleMetrics loop below. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java <https://reviews.apache.org/r/31107/#comment119503> I'd make these final too, while you're here. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java <https://reviews.apache.org/r/31107/#comment119504> This should do something other than swallow InterruptedException. See http://www.ibm.com/developerworks/library/j-jtp05236/ . exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java <https://reviews.apache.org/r/31107/#comment119506> Reuse the id variable from above (which looks like it should be made final). exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java <https://reviews.apache.org/r/31107/#comment119507> Could this be final? exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java <https://reviews.apache.org/r/31107/#comment119508> Call part.getStats() once at the beginning, and reuse the local throughout. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java <https://reviews.apache.org/r/31107/#comment119510> part.getStats() once. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java <https://reviews.apache.org/r/31107/#comment119511> Call Thread.currentThread() once at the beginning, and then reuse. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java <https://reviews.apache.org/r/31107/#comment119513> Would it make sense to restore the original thread name here? If so, wrap the iface.execute() in a try and restore the name in a finally. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java <https://reviews.apache.org/r/31107/#comment119514> Is IOException the best choice here? Would RuntimeException make more sense? Either way, would a message be helpful in case there are problems? How would a user know what to do if this exception fired? - Chris Westin On Feb. 17, 2015, 12:31 a.m., Yuliya Feldman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31107/ > ----------------------------------------------------------- > > (Updated Feb. 17, 2015, 12:31 a.m.) > > > Review request for drill, Chris Westin, Jacques Nadeau, Steven Phillips, and > Venki Korukanti. > > > Bugs: DRILL-2210 > https://issues.apache.org/jira/browse/DRILL-2210 > > > Repository: drill-git > > > Description > ------- > > In addition to description > > Fixed few classes that did not handle multithreading well > Added/Changed some Stats behavior to allow stats merge from multiple threads, > since again this class is not suitable to be used in multithreaded environment > Introduced new decorator class to handle multi thrteading (or not) to > minimize changes to ParitionSenderRootExec class > > > Diffs > ----- > > exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java > 0e9da0e > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java > 7af7b65 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java > f09acaa > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java > 5ed9c39 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java > 4292c09 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java > faa8546 > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java > aa0a5ad > > Diff: https://reviews.apache.org/r/31107/diff/ > > > Testing > ------- > > Still need to provide Unit Tests. > > Functional tests are passing > > Performance tests were run and look promising for some queries > > > Thanks, > > Yuliya Feldman > >
