ivoson commented on code in PR #39459:
URL: https://github.com/apache/spark/pull/39459#discussion_r1118005755


##########
core/src/main/scala/org/apache/spark/storage/BlockInfoManager.scala:
##########
@@ -399,7 +426,14 @@ private[storage] class BlockInfoManager extends Logging {
     try {
       val wrapper = new BlockInfoWrapper(newBlockInfo, lock)
       while (true) {
-        val previous = blockInfoWrappers.putIfAbsent(blockId, wrapper)
+        val previous = invisibleRDDBlocks.synchronized {
+          val res = blockInfoWrappers.putIfAbsent(blockId, wrapper)
+          if (res == null && trackingCacheVisibility) {
+            // Added to invisible blocks if it doesn't exist before.
+            blockId.asRDDId.foreach(invisibleRDDBlocks.add)

Review Comment:
   Currently we syncrhonized all the write operations, and also the read 
operations which need to check both the variables. For other read operations, 
since the variabled won't change, I think synchronized block for such read 
operations may be not necessary.



-- 
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: reviews-unsubscr...@spark.apache.org

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


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

Reply via email to