Ngone51 commented on a change in pull request #31876:
URL: https://github.com/apache/spark/pull/31876#discussion_r617593073



##########
File path: core/src/main/scala/org/apache/spark/scheduler/MapStatus.scala
##########
@@ -52,8 +53,20 @@ private[spark] sealed trait MapStatus {
    * partitionId of the task or taskContext.taskAttemptId is used.
    */
   def mapId: Long
-}
 
+  protected def loadLocation(in: ObjectInput): Location = {
+    val conf = SparkEnv.get.conf
+    conf.get(config.SHUFFLE_LOCATION_PLUGIN_CLASS).map { locClass =>
+      val loc = Utils.loadExtensions(
+        classOf[Location],
+        Seq(locClass),
+        conf
+      ).head
+      loc.readExternal(in)
+      loc

Review comment:
       By "fresh Location instance", I mean, we always need to create the 
instance using 0-arg constructor first (this's the "fresh" one) and then make 
it to a specific one by "readExternal".
   
   Follow your idea, I think we cache two things in `LocationFactory`:
   
   1) cache the loaded class of the custom location so we don't need to load it 
every time
   
   2) cache the location instances so we don't keep the references of multiple 
instances for the same location
   
   If so, I think we can do it by ourselves instead of exposing 
`LocationFactory` to users.
   
   Does it sound good to you?
   
   




-- 
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to