[SPARK-45338][CORE][SQL] Replace `scala.collection.JavaConverters` to `scala.jdk.CollectionConverters` #43126

Hisoka-X · 2023-09-26T10:54:14Z

What changes were proposed in this pull request?

Since scala 2.13.0, scala.collection.JavaConverters mark as deprecated. So this PR change all scala.collection.JavaConverters to scala.jdk.CollectionConverters.

Why are the changes needed?

Remove deprecated scala.collection.JavaConverters.

Does this PR introduce any user-facing change?

No

How was this patch tested?

passed CI

Was this patch authored or co-authored using generative AI tooling?

No

…cala.jdk.CollectionConverters

Hisoka-X · 2023-09-26T10:54:55Z

cc @dongjoon-hyun @LuciferYang

LuciferYang · 2023-09-26T10:55:41Z

also cc @srowen

LuciferYang · 2023-09-26T12:03:16Z

spark/scalastyle-config.xml

Lines 254 to 259 in 2aa06fc

    
           <!-- As of SPARK-9613 JavaConversions should be replaced with JavaConverters --> 
        
           <check customId="javaconversions" level="error" class="org.scalastyle.scalariform.TokenChecker" enabled="true"> 
        
             <parameters><parameter name="regex">JavaConversions</parameter></parameters> 
        
             <customMessage>Instead of importing implicits in scala.collection.JavaConversions._, import 
        
             scala.collection.JavaConverters._ and use .asScala / .asJava methods</customMessage> 
        
           </check>

Will this check rule be fixed in another pr? Or fixed together in this one?

Hisoka-X · 2023-09-26T12:07:20Z

spark/scalastyle-config.xml

Lines 254 to 259 in 2aa06fc



<check customId="javaconversions" level="error" class="org.scalastyle.scalariform.TokenChecker" enabled="true">

<parameters><parameter name="regex">JavaConversions</parameter></parameters>

<customMessage>Instead of importing implicits in scala.collection.JavaConversions._, import

scala.collection.JavaConverters._ and use .asScala / .asJava methods</customMessage>

</check>

Will this check rule be fixed in another pr? Or fixed together in this one?

Let me change it in this PR.

mllib/src/main/scala/org/apache/spark/mllib/api/python/GaussianMixtureModelWrapper.scala

LuciferYang · 2023-09-27T04:30:39Z

I move the parent to SPARK-45314

LuciferYang

Although the change involves many files, this is fine to me.

srowen · 2023-09-27T13:23:03Z

Merged to master

Hisoka-X · 2023-09-27T13:37:25Z

Thanks @LuciferYang @srowen

LuciferYang · 2023-09-28T09:22:59Z

mllib/src/main/scala/org/apache/spark/mllib/api/python/GaussianMixtureModelWrapper.scala

@@ -37,7 +37,7 @@ private[python] class GaussianMixtureModelWrapper(model: GaussianMixtureModel) {
    val modelGaussians = model.gaussians.map { gaussian =>
      Array[Any](gaussian.mu, gaussian.sigma)
    }
-    SerDe.dumps(JavaConverters.seqAsJavaListConverter(modelGaussians).asJava)
+    SerDe.dumps(modelGaussians.toSeq.asJava)


@Hisoka-X

On second thought, maybe we should replace modelGaussians.toSeq.asJava with immutable.ArraySeq.unsafeWrapArray(modelGaussians).asJava here, as modelGaussians.toSeq would result in an extra Array.copyOf, which is more memory-consuming.

WDYT @srowen

I'm not sure about we should do this, if we do that, should we change other toSeq? Which toSeq need to be modified and which ones do not? Are there any rules?

I'm inclined to optimize the several cases involved in this PR at #43172 first. For other similar issues, we can fix them together in the future. The current version has suppressed many compilation warnings, and if my memory serves me right, this includes parts related to this case.

LuciferYang · 2023-09-28T09:29:56Z

...tserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkGetColumnsOperation.scala

@@ -87,7 +87,7 @@ private[hive] class SparkGetColumnsOperation(
    }.toMap

    if (isAuthV2Enabled) {
-      val privObjs = seqAsJavaListConverter(getPrivObjs(db2Tabs)).asJava
+      val privObjs = getPrivObjs(db2Tabs).toSeq.asJava


this .toSeq is redundant

Let me fix it. Thanks for review again!

LuciferYang · 2023-09-28T09:30:46Z

...erver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkGetFunctionsOperation.scala

@@ -68,7 +68,7 @@ private[hive] class SparkGetFunctionsOperation(
    if (isAuthV2Enabled) {
      // authorize this call on the schema objects
      val privObjs =
-        HivePrivilegeObjectUtils.getHivePrivDbObjects(seqAsJavaListConverter(matchingDbs).asJava)
+        HivePrivilegeObjectUtils.getHivePrivDbObjects(matchingDbs.toSeq.asJava)


this .toSeq is redundant

### What changes were proposed in this pull request? This is a follow up PR for #43126 , remove useless invoke `toSeq` ### Why are the changes needed? Remove useless convert. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? exist test ### Was this patch authored or co-authored using generative AI tooling? No Closes #43172 from Hisoka-X/SPARK-45338-followup-remove-toseq. Authored-by: Jia Fan <fanjiaeminem@qq.com> Signed-off-by: Sean Owen <srowen@gmail.com>

…WrapArray` instead of `array.toSeq` ### What changes were proposed in this pull request? In #43126, there is a change from `JavaConverters.seqAsJavaListConverter(array).asJava` to `array.toSeq.asJava`. Since `array.toSeq` will involve an extra `Array.copyOf`, and the involved scenario in the subsequent processing is to directly convert the result of `array.toSeq.asJava` into an `Array[Byte]` result without the possibility of modifying the array content, so this pr replaces `array.toSeq` with `immutable.ArraySeq.unsafeWrapArray(array)` to avoid the extra `Array.copyOf`. ### Why are the changes needed? In the current scenario, there is no risk of array data being modified, and `immutable.ArraySeq.unsafeWrapArray(array)` compared to `array.toSeq` avoids an extra `Array.copyOf`, bringing certain performance benefits and reducing memory usage. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Pass GitHub Actions ### Was this patch authored or co-authored using generative AI tooling? No Closes #43171 from LuciferYang/SPARK-45338-FOLLOWUP. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

beliefer · 2023-10-08T11:05:53Z

@Hisoka-X Thank you! I got the change.

Hisoka-X added 2 commits September 26, 2023 18:48

[SPARK-45338][CORE][SQL] Replace scala.collection.JavaConverters to s…

50fe82e

…cala.jdk.CollectionConverters

update

01ed5d9

github-actions bot added SQL ML MLLIB STRUCTURED STREAMING KUBERNETES WEB UI MESOS BUILD YARN EXAMPLES CORE PYTHON R DSTREAM AVRO CONNECT PROTOBUF labels Sep 26, 2023

Hisoka-X added 2 commits September 26, 2023 20:09

update

2d86bf1

format

527c4bc

LuciferYang reviewed Sep 26, 2023

View reviewed changes

mllib/src/main/scala/org/apache/spark/mllib/api/python/GaussianMixtureModelWrapper.scala Outdated Show resolved Hide resolved

fix review

a509d06

LuciferYang approved these changes Sep 27, 2023

View reviewed changes

srowen closed this in b7763a7 Sep 27, 2023

Hisoka-X deleted the replace-java-converter branch September 27, 2023 13:38

LuciferYang reviewed Sep 28, 2023

View reviewed changes

Hisoka-X mentioned this pull request Sep 28, 2023

[SPARK-45338][SQL][FOLLOWUP] Remove useless toSeq #43172

Closed

LuciferYang mentioned this pull request Sep 29, 2023

[SPARK-45338][MLLIB][PYTHON][FOLLOWUP] Use immutable.ArraySeq.unsafeWrapArray instead of array.toSeq #43171

Closed

panbingkun mentioned this pull request Oct 8, 2023

[SPARK-45460][SQL] Replace scala.collection.convert.ImplicitConversions to scala.jdk.CollectionConverters #43275

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-45338][CORE][SQL] Replace `scala.collection.JavaConverters` to `scala.jdk.CollectionConverters` #43126

[SPARK-45338][CORE][SQL] Replace `scala.collection.JavaConverters` to `scala.jdk.CollectionConverters` #43126

Hisoka-X commented Sep 26, 2023

Hisoka-X commented Sep 26, 2023

LuciferYang commented Sep 26, 2023

LuciferYang commented Sep 26, 2023

Hisoka-X commented Sep 26, 2023

LuciferYang commented Sep 27, 2023

LuciferYang left a comment •

edited

srowen commented Sep 27, 2023

Hisoka-X commented Sep 27, 2023

LuciferYang Sep 28, 2023

Hisoka-X Sep 28, 2023

LuciferYang Sep 28, 2023 •

edited

LuciferYang Sep 28, 2023

Hisoka-X Sep 28, 2023

LuciferYang Sep 28, 2023

beliefer commented Oct 8, 2023

[SPARK-45338][CORE][SQL] Replace scala.collection.JavaConverters to scala.jdk.CollectionConverters #43126

[SPARK-45338][CORE][SQL] Replace scala.collection.JavaConverters to scala.jdk.CollectionConverters #43126

Conversation

Hisoka-X commented Sep 26, 2023

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Hisoka-X commented Sep 26, 2023

LuciferYang commented Sep 26, 2023

LuciferYang commented Sep 26, 2023

Hisoka-X commented Sep 26, 2023

LuciferYang commented Sep 27, 2023

LuciferYang left a comment • edited

Choose a reason for hiding this comment

srowen commented Sep 27, 2023

Hisoka-X commented Sep 27, 2023

LuciferYang Sep 28, 2023

Choose a reason for hiding this comment

Hisoka-X Sep 28, 2023

Choose a reason for hiding this comment

LuciferYang Sep 28, 2023 • edited

Choose a reason for hiding this comment

LuciferYang Sep 28, 2023

Choose a reason for hiding this comment

Hisoka-X Sep 28, 2023

Choose a reason for hiding this comment

LuciferYang Sep 28, 2023

Choose a reason for hiding this comment

beliefer commented Oct 8, 2023

[SPARK-45338][CORE][SQL] Replace `scala.collection.JavaConverters` to `scala.jdk.CollectionConverters` #43126

[SPARK-45338][CORE][SQL] Replace `scala.collection.JavaConverters` to `scala.jdk.CollectionConverters` #43126

LuciferYang left a comment •

edited

LuciferYang Sep 28, 2023 •

edited