zhipeng93 commented on code in PR #212:
URL: https://github.com/apache/flink-ml/pull/212#discussion_r1112795550


##########
flink-ml-benchmark/src/main/java/org/apache/flink/ml/benchmark/datagenerator/common/DenseVectorArrayGenerator.java:
##########
@@ -42,7 +42,7 @@ protected RowGenerator[] getRowGenerators() {
         return new RowGenerator[] {
             new RowGenerator(getNumValues(), getSeed()) {
                 @Override
-                protected Row nextRow() {
+                protected Row getRow() {

Review Comment:
   Reusing one row to simulate the input may cause the benchmark result not 
stable, since processing two different rows may take different time.
   
   The two optimizations introduced in this PR give us 12% speedup. Is there a 
breakdown analysis of this two optimization?
   
   If the data generation cost is indeed high, can we sample a fixed number of 
rows and reuse it in the evalution?



##########
flink-ml-benchmark/src/main/java/org/apache/flink/ml/benchmark/datagenerator/common/DenseVectorArrayGenerator.java:
##########
@@ -42,7 +42,7 @@ protected RowGenerator[] getRowGenerators() {
         return new RowGenerator[] {
             new RowGenerator(getNumValues(), getSeed()) {
                 @Override
-                protected Row nextRow() {
+                protected Row getRow() {

Review Comment:
   Reusing one row to simulate the input may cause the benchmark result not 
stable, since processing two different rows may take different time.
   
   The two optimizations introduced in this PR give us 12% speedup. Is there a 
breakdown analysis of this two optimization?
   
   If the data generation cost is indeed high, can we sample a fixed number of 
rows and reuse it in the evaluation?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to