-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Review/fix definition of IArray #11298
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
Comments
I was planning on teaching |
|
Actually, access to the array does not need reflection. It only has to load the module and call a method. Inlining would still help remove the module instantiation and method call. Example 1class E:
def test1 =
val a = Array(1)
a(0)
def test2 =
val a = IArray(1)
a(0) public class E {
public E();
Code:
0: aload_0
1: invokespecial #14 // Method java/lang/Object."<init>":()V
4: return
public int test1();
Code:
0: iconst_1
1: newarray int
3: dup
4: iconst_0
5: iconst_1
6: iastore
7: astore_1
8: aload_1
9: iconst_0
10: iaload
11: ireturn
public int test2();
Code:
0: iconst_1
1: newarray int
3: dup
4: iconst_0
5: iconst_1
6: iastore
7: astore_1
8: getstatic #25 // Field scala/opaques$arrayOps$.MODULE$:Lscala/opaques$arrayOps$;
11: aload_1
12: iconst_0
13: invokevirtual #29 // Method scala/opaques$arrayOps$.apply:([II)I
16: ireturn
} Example 2class E:
def test1 =
val a = Array(1)
a(0)
def test2 =
val a = IArray(1)
a(0)
extension (arr: IArray[Int]) def apply(n: Int): Int = arr.asInstanceOf[Array[Int]].apply(n) public class E {
public E();
Code:
0: aload_0
1: invokespecial #9 // Method java/lang/Object."<init>":()V
4: return
public int test1();
Code:
0: iconst_1
1: newarray int
3: dup
4: iconst_0
5: iconst_1
6: iastore
7: astore_1
8: aload_1
9: iconst_0
10: iaload
11: ireturn
public int test2();
Code:
0: iconst_1
1: newarray int
3: dup
4: iconst_0
5: iconst_1
6: iastore
7: astore_1
8: aload_0
9: aload_1
10: iconst_0
11: invokevirtual #20 // Method apply:([II)I
14: ireturn
public int apply(int[], int);
Code:
0: aload_1
1: iload_2
2: iaload
3: ireturn
} |
@nicolasstucki What about the other operations? They all go through genericArrayOps. Would that not slow things down? |
@propensive There is a type alias in |
@nicolasstucki Yes, I think most missing methods come from |
We implemented the operations of We are missing |
@odersky That's perfect then. My guess is that beginners wouldn't focus too much on the package name if they don't have to type it. |
What is the purpose of putting it in a different package? An if it should, |
I’m not a big fan of the Do we want to deprecate |
@julienrf |
The missing methods can be added with #11319 |
#11329 seems to provide a good alternative. |
It would be good to do some micro-benchmarks about performance of array operations vs IArray operations. Ideally the performance should be the same after warmup. We should test direct access with apply as well as common collection operations such as map, flatMap, filter, foldLeft. And we should test with elements of both primitive and reference types. |
IArray
needs to be reviewed and tested before we can release it.Right now, I see see several issues:
toList
is not defined, but there are many others.IArray[A]
is backed byArray[A]
every access tothe underlying array goes through reflection. We need to find a way to inline the access
methods. This would be easier if we can expand opaque types before pickling since
then inline methods and opaque types can co-exist.
scala.opaques.IArray
, or is something else better?The text was updated successfully, but these errors were encountered: