Small optimisations - some faster array copies, reuse empty arrays by mkeskells · Pull Request #9091 · scala/scala

mkeskells · 2020-06-28T15:31:20Z

take advantage of JVM optimisation when copying arrays to avoid zeroing

reuse empty arrays where it is simple to do so

mkeskells · 2020-06-28T21:46:16Z

/rebuild

src/library/scala/Array.scala

…ntent of the previous array is to be copied into the new Array Use clone when the array is not changing length Maintain empty Arrays in the Array object, and use them where possible Maintain empty WrappedArrays in WrappedArrays object and use where possible

mkeskells · 2020-07-02T22:25:33Z

Mima issues

static method emptyUnitArray()Array[scala.runtime.BoxedUnit] in class scala.Array does not have a correspondent in other version
filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("scala.Array.emptyUnitArray")
method emptyUnitArray()Array[scala.runtime.BoxedUnit] in object scala.Array does not have a correspondent in other version
filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("scala.Array.emptyUnitArray")
object scala.collection.mutable.WrappedArrayBuilder does not have a correspondent in other version
filter with: ProblemFilters.exclude[MissingClassProblem]("scala.collection.mutable.WrappedArrayBuilder$")

lrytz

LGTM otherwise

src/library/scala/collection/mutable/WrappedArrayBuilder.scala

lrytz

Looking at this again made me think about the two new ClassValue caches. In a quick internal discussion, @retronym suggested to add new, lazily instantiated fields to ClassTag instead. It would work out well here because both of the new ClassValue caches are used in places where a ClassTag is available. I like this idea and think we should do it to avoid the two new caches.

mkeskells · 2020-08-15T15:17:34Z

Looking at this again made me think about the two new ClassValue caches. In a quick internal discussion, @retronym suggested to add new, lazily instantiated fields to ClassTag instead. It would work out well here because both of the new ClassValue caches are used in places where a ClassTag is available. I like this idea and think we should do it to avoid the two new caches.

Added a commit with the changes.

…edArray

mkeskells · 2020-08-18T07:30:08Z

MiMa issues (emptyArray and emptyWrappedArray are private[scala])

    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ClassTag#GenericClassTag.emptyArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ClassTag#GenericClassTag.emptyWrappedArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ManifestFactory#IntersectionTypeManifest.emptyArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ManifestFactory#IntersectionTypeManifest.emptyWrappedArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ManifestFactory#ClassTypeManifest.emptyArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ManifestFactory#ClassTypeManifest.emptyWrappedArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.AnyValManifest.emptyArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.AnyValManifest.emptyWrappedArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ClassTag.emptyArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ClassTag.emptyWrappedArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ManifestFactory#WildcardManifest.emptyArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ManifestFactory#WildcardManifest.emptyWrappedArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ManifestFactory#SingletonTypeManifest.emptyArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ManifestFactory#SingletonTypeManifest.emptyWrappedArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ClassManifestFactory#AbstractTypeClassManifest.emptyArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ClassManifestFactory#AbstractTypeClassManifest.emptyWrappedArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ManifestFactory#AbstractTypeManifest.emptyArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ManifestFactory#AbstractTypeManifest.emptyWrappedArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ClassTypeManifest.emptyArray"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.ClassTypeManifest.emptyWrappedArray"),

lrytz

LGTM. I pushed a small cleanup for the mima filters.

In a follow-up, you could use the cached empty instance in ClassTag.newArray for primitives (you only do it for references):

scala> ClassTag.Int.newArray(0) eq ClassTag.Int.newArray(0)
res18: Boolean = false

scala> implicitly[ClassTag[String]].newArray(0) eq implicitly[ClassTag[String]].newArray(0)
res19: Boolean = true

dwijnand · 2020-09-02T09:44:21Z

Impacted Scala.js https://travis-ci.org/github/scala-js/scala-js-test-scala-nightly/jobs/723284403, chat on Gitter: https://gitter.im/scala/contributors?at=5f4f60db48237809375c2254 (intertwined with a chat about Shapeless)

this regressed in scala#9091 the problem turned up in the Scala 2.12 community build -- when Scala 3 support was added to the scala-collection-compat repo, a test case there was altered in a way that happened to tickle this

SethTisue · 2020-11-21T21:04:27Z

#9335 fixes a regression here (in Array.empty[Unit])

Problem ======= Upgrade source to https://github.com/scala/scala/releases/tag/v2.12.13 from https://github.com/scala/scala/releases/tag/v2.12.12. This upgrade includes a highlighted feature of configurable warnings and errors (https://www.scala-lang.org/2021/01/12/configuring-and-suppressing-warnings.html / scala/scala#9248). Other noticable changes our code based will experience * Exhaustivity warnings for patterns involving tuples * Better type checking for `CharSequence` arguments scala/scala#9292 * Simplified Reporters scala/scala#8338 * Promotion of `-deprecation` to `-Xlint:deprecation` scala/scala#7714 * Improves performance of building `immutable.{TreeMap,TreeSet}` by using mutation within the builder scala/scala#8794 Full list of changes can be found at https://github.com/scala/scala/pulls?q=is%3Amerged+milestone%3A2.12.13+ Solution ======== * Bump `BUILD` file `SCALA_REV` && `SCALAC_REV_FOR_DEPS`. * Depdnencies which are not built for scala 2.12.13 needed to be bumped `SEMANTICDB_PLUGIN_REV` && `SEMANTICDB_REV` && `SCALAFIX_REV`. * Include `-S-Xlint:-deprecation` to `pants.ini` preventing build failures on deprecated annotations (existing behavior) * Bump `cache_key_gen_version` in `pants.ini` for newly built artifacts on different scala version. * Removed scalafix plugin (`scalac-profiling`) which is not built for 2.12.13. `scalac-profiling` code looks abandoned by ~3 years. * Updated all failing tests that could have depended or expected ordered sequences when the sequence was generated from non ordered collections. Notes ===== It seems a few tests and golden files in source have depended or tested against a stable sort order when sorted order is not guaranteed. This has been seen in a few places such as output json objects (map key fields), code that uses `groupBy` for rekeying results to the user and transforming into sequences, strings built from sequences or maps that are not guaranteed to be ordered. The following PR are related to this change in expectations scala/scala#8794 scala/scala#8948 scala/scala#9218 scala/scala#9376 scala/scala#9091 scala/scala#9216 We took the liberty updating tests with what the current map order would be or updating the test in a way that wouldn't depend on order. Since we may not fully understand all the context of the tests we are hoping this either signals to the owners that there might be some issue with assumed order or signal that upgrading might break implementations due to bug in scala 2.12.13. {D611202} will improve the files changed for workflow builder outside of this change. Please feel to reach out directly and discuss the changes here or bring up issues with this upgrade. Slack [[https://app.slack.com/client/T86S8GHEG/C01NZAFRLFK|#scala-upgrade-2-12-13]] JIRA Issues: SCALA-25 Differential Revision: https://phabricator.twitter.biz/D607826

Small optimisations - some faster array copies, reuse empty arrays

this regressed in scala/scala#9091 the problem turned up in the Scala 2.12 community build -- when Scala 3 support was added to the scala-collection-compat repo, a test case there was altered in a way that happened to tickle this

Small optimisations - some faster array copies, reuse empty arrays

this regressed in scala/scala#9091 the problem turned up in the Scala 2.12 community build -- when Scala 3 support was added to the scala-collection-compat repo, a test case there was altered in a way that happened to tickle this

scala-jenkins added this to the 2.13.4 milestone Jun 28, 2020

mkeskells changed the base branch from 2.13.x to 2.12.x June 28, 2020 15:32

lrytz reviewed Jun 29, 2020

View reviewed changes

src/library/scala/Array.scala Outdated Show resolved Hide resolved

mkeskells force-pushed the 2.12.x_array_copy branch 3 times, most recently from b10978e to 3e2dbe0 Compare July 2, 2020 17:15

mkeskells force-pushed the 2.12.x_array_copy branch from 3e2dbe0 to 1fb249c Compare July 2, 2020 22:25

diesalbla added the library:collections PRs involving changes to the standard collection library label Jul 3, 2020

lrytz reviewed Jul 8, 2020

View reviewed changes

src/library/scala/collection/mutable/WrappedArrayBuilder.scala Outdated Show resolved Hide resolved

simplify signature

7bba932

mkeskells requested a review from lrytz August 2, 2020 15:30

mkeskells modified the milestones: 2.13.4, 2.12.13 Aug 10, 2020

lrytz requested changes Aug 13, 2020

View reviewed changes

mkeskells force-pushed the 2.12.x_array_copy branch 2 times, most recently from e9f4603 to eeb2e7c Compare August 14, 2020 17:09

mkeskells force-pushed the 2.12.x_array_copy branch 2 times, most recently from cb88d1b to b2e4532 Compare August 17, 2020 16:10

review feedback - use the ClassTag to hold the empty Arrays and Wrapp…

31d6481

…edArray

mkeskells force-pushed the 2.12.x_array_copy branch from b2e4532 to 31d6481 Compare August 18, 2020 07:29

mkeskells requested a review from lrytz August 19, 2020 11:52

simplify mima exceptions

6ae4136

lrytz approved these changes Aug 20, 2020

View reviewed changes

lrytz merged commit 7522126 into scala:2.12.x Aug 20, 2020

SethTisue added the performance the need for speed. usually compiler performance, sometimes runtime performance. label Aug 20, 2020

This was referenced Sep 15, 2020

Merge 2.12.x into 2.13.x [ci: last-only] #9206

Merged

PRs skipped when branch merging for 2.13.4 scala/scala-dev#721

Closed

SethTisue mentioned this pull request Nov 21, 2020

fix never-released 2.12.13 regression where Array[Unit].empty caused NPE #9335

Merged

This was referenced Jun 9, 2021

Update sbt-scalajs, ... to 1.6.0 scala/scala-collection-compat#462

Closed

1.6.0 regression: new mutable.WrappedArrayBuilder(classTag[Unit]).result() throws a CCE scala-js/scala-js#4507

Closed

hamzaremmal pushed a commit to hamzaremmal/scala3 that referenced this pull request May 2, 2025

Merge pull request scala/scala#9091 from mkeskells/2.12.x_array_copy

868c881

Small optimisations - some faster array copies, reuse empty arrays

hamzaremmal pushed a commit to scala/scala3 that referenced this pull request May 7, 2025

Merge pull request scala/scala#9091 from mkeskells/2.12.x_array_copy

f97da22

Small optimisations - some faster array copies, reuse empty arrays

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Small optimisations - some faster array copies, reuse empty arrays#9091

Small optimisations - some faster array copies, reuse empty arrays#9091
lrytz merged 4 commits intoscala:2.12.xfrom
mkeskells:2.12.x_array_copy

mkeskells commented Jun 28, 2020

Uh oh!

mkeskells commented Jun 28, 2020

Uh oh!

Uh oh!

mkeskells commented Jul 2, 2020

Uh oh!

lrytz left a comment

Uh oh!

Uh oh!

lrytz left a comment

Uh oh!

mkeskells commented Aug 15, 2020

Uh oh!

mkeskells commented Aug 18, 2020 •

edited

Loading

Uh oh!

lrytz left a comment

Uh oh!

dwijnand commented Sep 2, 2020 •

edited

Loading

Uh oh!

SethTisue commented Nov 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

mkeskells commented Jun 28, 2020

Uh oh!

mkeskells commented Jun 28, 2020

Uh oh!

Uh oh!

mkeskells commented Jul 2, 2020

Uh oh!

lrytz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lrytz left a comment

Choose a reason for hiding this comment

Uh oh!

mkeskells commented Aug 15, 2020

Uh oh!

mkeskells commented Aug 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lrytz left a comment

Choose a reason for hiding this comment

Uh oh!

dwijnand commented Sep 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SethTisue commented Nov 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mkeskells commented Aug 18, 2020 •

edited

Loading

dwijnand commented Sep 2, 2020 •

edited

Loading