[GitHub] [incubator-uniffle] smallzhongfeng commented on a diff in pull request #637: [#615] improvement: Reduce task binary by removing 'partitionToServers' from RssShuffleHandle

["smallzhongfeng (via GitHub)" <gi...@apache.org>]

smallzhongfeng commented on code in PR #637:
URL: https://github.com/apache/incubator-uniffle/pull/637#discussion_r1125418456


##########
client-spark/common/src/main/java/org/apache/spark/shuffle/RssShuffleHandle.java:
##########
@@ -32,29 +37,27 @@
   private String appId;
   private int numMaps;
   private ShuffleDependency<K, V, C> dependency;
-  private Map<Integer, List<ShuffleServerInfo>> partitionToServers;
-  // shuffle servers which is for store shuffle data
-  private Set<ShuffleServerInfo> shuffleServersForData;
-  // remoteStorage used for this job
-  private RemoteStorageInfo remoteStorage;
+  private Broadcast<byte[]> handlerInfoBytesBd;
+
+  // shuffle ID to ShuffleIdRef
+  // ShuffleIdRef acts as strong reference to prevent cached ShuffleHandleInfo being GCed during shuffle
+  // ShuffleIdRef will be removed when unregisterShuffle()
+  private static Map<Integer, ShuffleIdRef> _globalShuffleIdRefMap = new ConcurrentHashMap<>();
+  // each shuffle has unique ID even for multiple concurrent running shuffles and jobs per application
+  private static ThreadLocal<HandleInfoLocalCache> _localHandleInfoCache =

Review Comment:
   ditto.



##########
client-spark/common/src/main/java/org/apache/spark/shuffle/RssShuffleHandle.java:
##########
@@ -32,29 +37,27 @@
   private String appId;
   private int numMaps;
   private ShuffleDependency<K, V, C> dependency;
-  private Map<Integer, List<ShuffleServerInfo>> partitionToServers;
-  // shuffle servers which is for store shuffle data
-  private Set<ShuffleServerInfo> shuffleServersForData;
-  // remoteStorage used for this job
-  private RemoteStorageInfo remoteStorage;
+  private Broadcast<byte[]> handlerInfoBytesBd;
+
+  // shuffle ID to ShuffleIdRef
+  // ShuffleIdRef acts as strong reference to prevent cached ShuffleHandleInfo being GCed during shuffle
+  // ShuffleIdRef will be removed when unregisterShuffle()
+  private static Map<Integer, ShuffleIdRef> _globalShuffleIdRefMap = new ConcurrentHashMap<>();

Review Comment:
   It seems that the underline does not conform to the name definition specification of `java`. Right?



-- 
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-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org