Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-39056][CORE][SQL] Use Collections.singletonList instead of Arrays.asList when only one argument #36393

Conversation

LuciferYang
Copy link
Contributor

What changes were proposed in this pull request?

This is pr use Collections.singletonList instead of Arrays.asList when there is only one argument.

Before

List<String> one = Arrays.asList("one"); 

After

List<String> one = Collections.singletonList("one"); 

Why are the changes needed?

A minor optimization will save a little memory due to Arrays.asList will create an E[] but Collections.SingletonList not.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass GA

@LuciferYang LuciferYang marked this pull request as draft April 28, 2022 12:21
Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is worth doing, it just doesn't save any memory

@LuciferYang
Copy link
Contributor Author

OK~ close this

@LuciferYang
Copy link
Contributor Author

I'm not sure this is worth doing, it just doesn't save any memory

Do a simple size test

    println(s"Size of Arrays.asList(1) is ${SizeEstimator.estimate(java.util.Arrays.asList(1))}")
    println(s"Size of Collections.singletonList(1) is " +
      s"${SizeEstimator.estimate(java.util.Collections.singletonList(1))}")

The results as follows:

Size of Arrays.asList(1) is 64
Size of Collections.singletonList(1) is 40

And for performance

val valuesPerIteration = 1000000
val benchmark = new Benchmark(
      s"Test singletonList",
      valuesPerIteration,
      output = output)

    benchmark.addCase("Arrays.asList") { _: Int =>
      for (i <- 0L until valuesPerIteration) {
        java.util.Arrays.asList(i)
      }
    }

    benchmark.addCase("Collections.singletonList") { _: Int =>
      for (i <- 0L until valuesPerIteration) {
        java.util.Collections.singletonList(i)
      }
    }
    benchmark.run()

The results as follows:

OpenJDK 64-Bit Server VM 1.8.0_322-b06 on Mac OS X 11.4
Apple M1
Test singletonList:                       Best Time(ms)   Avg Time(ms)   Stdev(ms)    Rate(M/s)   Per Row(ns)   Relative
------------------------------------------------------------------------------------------------------------------------
Arrays.asList                                         5              5           0        213.4           4.7       1.0X
Collections.singletonList                             4              4           0        252.7           4.0       1.2X

Although the advantage is very small, it is good for performance and memory

@srowen
Copy link
Member

srowen commented Apr 28, 2022

As used, 24 bytes in 1 allocation doesn't matter. These aren't in tight loops

@LuciferYang
Copy link
Contributor Author

Got it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants