spark-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [spark] JoshRosen commented on a change in pull request #27246: [SPARK-30536][CORE][SQL] Sort-merge join operator spilling performance improvements
Date Thu, 23 Jan 2020 03:35:13 GMT
JoshRosen commented on a change in pull request #27246: [SPARK-30536][CORE][SQL] Sort-merge
join operator spilling performance improvements
URL: https://github.com/apache/spark/pull/27246#discussion_r369918625
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/ExternalAppendOnlyUnsafeRowArray.scala
 ##########
 @@ -106,6 +107,8 @@ private[sql] class ExternalAppendOnlyUnsafeRowArray(
   def add(unsafeRow: UnsafeRow): Unit = {
     if (numRows < numRowsInMemoryBufferThreshold) {
       inMemoryBuffer += unsafeRow.copy()
+      numRows += 1
 
 Review comment:
   I think that we need to increment `numRows` and `modificationsCount` in both branches,
not just this one: if we only update it in this branch then `length()` will return the wrong
result.
   
   If you want to know how many rows are in the in-memory buffer then I'd define a separate
`numRowBufferedInMemory` variable; that will also help to make the code in `MergerIterator`
easier to understand.
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Mime
View raw message