Avoid IntRef and other overheads in `ScalaRuntime.{toArray, toObjectArray}` by mkeskells · Pull Request #8688 · scala/scala

mkeskells · 2020-02-03T12:01:16Z

No description provided.

lrytz · 2020-02-03T12:29:24Z

src/library/scala/runtime/ScalaRunTime.scala

+        }
+      }
+      xs foreach copier
+      copier.arr


Using an iterator seems more natural - is there a reason not to?

That does seem unnatural. But I'm reminded there is still no lint for Ref creation.

I just wrote it is a for loop, because the original was like that. I am not sure where this is implicitly called from. I have changed it to an iterator

Regarding the Ref creation - there are 152 matches for var .*\n[\W]*for regex in the codebase. Some of these are in comments and false matches, but there are lots of patterns that it doesn't pick up
Most of the cases that I see in library and compiler are ref creation.

We just did this case because we see a few 00Gb of IntRef, BooleanRef and ObjectRef and trying to squash them

@som-snytt I'm reminded people keep asking for one. #8691

hrhino

What's a few hundred gigs between friends?

dwijnand · 2020-02-03T22:33:05Z

Isn't storage cheap? Oh, this is memory. Then I guess: are you saying 640 K wasn't enough?

som-snytt · 2020-02-03T22:39:05Z

I changed my previous vote from heart to rocket.

hrhino · 2020-02-03T22:57:21Z

👀 as in roll.

retronym · 2020-02-04T06:28:48Z

👀 as in roll.

🙄 as in, "that's just the way 🙄 "

src/library/scala/runtime/ScalaRunTime.scala

lrytz · 2020-02-13T10:39:38Z

@mkeskells I couldn't find any call to the toArray method that you're optimizing, or any place in the compiler where we generate a call to this method. How did you observe it?

We do emit calls to toObjectArray (which should probably be optimzied in the same way, as suggested by Jason).

Instead, extends the type-case on the src array to separately deal with each type of primitive array, which can call into a specialized variant of the loop. I've also added a special case for empty arrays. This compiles as: ``` // access flags 0x1A private final static copy$mCc$sp$1([C)[Ljava/lang/Object; // parameter final src ALOAD 0 ARRAYLENGTH ISTORE 1 ILOAD 1 ICONST_0 IF_ICMPNE L0 GETSTATIC scala/Array$.MODULE$ : Lscala/Array$; INVOKEVIRTUAL scala/Array$.emptyObjectArray ()[Ljava/lang/Object; ARETURN L0 ILOAD 1 ANEWARRAY java/lang/Object ASTORE 2 ICONST_0 ISTORE 3 L1 ILOAD 3 ILOAD 1 IF_ICMPGE L2 ALOAD 2 ILOAD 3 ALOAD 0 ILOAD 3 CALOAD INVOKESTATIC scala/runtime/BoxesRunTime.boxToCharacter (C)Ljava/lang/Character; AASTORE ILOAD 3 ICONST_1 IADD ISTORE 3 GOTO L1 L2 ALOAD 2 ARETURN MAXSTACK = 4 MAXLOCALS = 4 ```

retronym · 2020-02-17T02:15:05Z

I've pushed a change to toObjectArray to signficantly tighten up the code we generate.

Now:

  // access flags 0x1A
  private final static copy$mBc$sp$1([B)[Ljava/lang/Object;
    // parameter final  src
    ALOAD 0
    ARRAYLENGTH
    ISTORE 1
    ILOAD 1
    ICONST_0
    IF_ICMPNE L0
    GETSTATIC scala/Array$.MODULE$ : Lscala/Array$;
    INVOKEVIRTUAL scala/Array$.emptyObjectArray ()[Ljava/lang/Object;
    ARETURN
   L0
    ILOAD 1
    ANEWARRAY java/lang/Object
    ASTORE 2
    ICONST_0
    ISTORE 3
   L1
    ILOAD 3
    ILOAD 1
    IF_ICMPGE L2
    ALOAD 2
    ILOAD 3
    ALOAD 0
    ILOAD 3
    BALOAD
    INVOKESTATIC scala/runtime/BoxesRunTime.boxToByte (B)Ljava/lang/Byte;
    AASTORE
    ILOAD 3
    ICONST_1
    IADD
    ISTORE 3
    GOTO L1
   L2
    ALOAD 2
    ARETURN
    MAXSTACK = 4
    MAXLOCALS = 4

  public toObjectArray(Ljava/lang/Object;)[Ljava/lang/Object;
    // parameter final  src
    ALOAD 1
    INSTANCEOF [Ljava/lang/Object;
    IFEQ L0
    ALOAD 1
    CHECKCAST [Ljava/lang/Object;
    ASTORE 2
    GOTO L1
   L0
    ALOAD 1
    INSTANCEOF [I
    IFEQ L2
    ALOAD 1
    CHECKCAST [I
    INVOKESTATIC scala/runtime/ScalaRunTime$.copy$mIc$sp$1 ([I)[Ljava/lang/Object;
    ASTORE 2
    GOTO L1
   L2
    ALOAD 1
    INSTANCEOF [D
    IFEQ L3
    ALOAD 1
    CHECKCAST [D
    INVOKESTATIC scala/runtime/ScalaRunTime$.copy$mDc$sp$1 ([D)[Ljava/lang/Object;
    ASTORE 2
    GOTO L1
   L3
    ALOAD 1
    INSTANCEOF [J
    IFEQ L4
    ALOAD 1
    CHECKCAST [J
    INVOKESTATIC scala/runtime/ScalaRunTime$.copy$mJc$sp$1 ([J)[Ljava/lang/Object;
    ASTORE 2
    GOTO L1
   L4
  ...

retronym · 2020-02-17T05:26:08Z

@lrytz Could you please review my additional commits?

lrytz · 2020-02-17T15:47:52Z

They LGTM. I still wonder about how the toArray method is actually used (#8688 (comment)) - @mkeskells do you know?

Avoid IntRef and other overheads in `ScalaRuntime.{toArray, toObjectArray}`

scala-jenkins added this to the 2.12.12 milestone Feb 3, 2020

lrytz reviewed Feb 3, 2020

View reviewed changes

don't create an IntRef in toArray

2ef4752

mkeskells force-pushed the mike/2.12.x_toArray branch from 3e29f4e to 2ef4752 Compare February 3, 2020 20:03

hrhino approved these changes Feb 3, 2020

View reviewed changes

retronym changed the title ~~dont create an IntRef in toArray~~ [nomerge] dont create an IntRef in toArray Feb 4, 2020

retronym changed the title ~~[nomerge] dont create an IntRef in toArray~~ dont create an IntRef in toArray Feb 4, 2020

retronym reviewed Feb 4, 2020

View reviewed changes

src/library/scala/runtime/ScalaRunTime.scala Show resolved Hide resolved

lrytz self-assigned this Feb 13, 2020

retronym added 2 commits February 17, 2020 12:13

Use a while loop in runtime support for varargs array conversions

e6befa2

retronym changed the title ~~dont create an IntRef in toArray~~ Avoid IntRef and other overheads in ScalaRuntime.{toArray, toObjectArray} Feb 17, 2020

SethTisue added the performance the need for speed. usually compiler performance, sometimes runtime performance. label Feb 18, 2020

retronym merged commit cf929d6 into scala:2.12.x Feb 25, 2020

lrytz modified the milestones: 2.12.12, 2.12.11 Feb 25, 2020

lrytz added the library:collections PRs involving changes to the standard collection library label Mar 16, 2020

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

Merge pull request scala/scala#8688 from rorygraves/mike/2.12.x_toArray

07d9091

Avoid IntRef and other overheads in `ScalaRuntime.{toArray, toObjectArray}`

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

Merge pull request scala/scala#8688 from rorygraves/mike/2.12.x_toArray

f354567

Avoid IntRef and other overheads in `ScalaRuntime.{toArray, toObjectArray}`

Conversation

mkeskells commented Feb 3, 2020

Uh oh!

lrytz Feb 3, 2020

Choose a reason for hiding this comment

Uh oh!

som-snytt Feb 3, 2020

Choose a reason for hiding this comment

Uh oh!

mkeskells Feb 3, 2020

Choose a reason for hiding this comment

Uh oh!

hrhino Feb 3, 2020

Choose a reason for hiding this comment

Uh oh!

hrhino left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

som-snytt commented Feb 3, 2020

Uh oh!

hrhino commented Feb 3, 2020

Uh oh!

retronym commented Feb 4, 2020

Uh oh!

Uh oh!

lrytz commented Feb 13, 2020

Uh oh!

retronym commented Feb 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

retronym commented Feb 17, 2020

Uh oh!

lrytz commented Feb 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

dwijnand commented Feb 3, 2020 •

edited

Loading

retronym commented Feb 17, 2020 •

edited

Loading