locationtech / geotrellis

GeoTrellis is a geographic data processing engine for high performance applications.
http://geotrellis.io
Other
1.33k stars 360 forks source link

ShortUserDefinedNoDataArrayTile.cols()/rows() calls scala/runtime/BoxesRunTime.boxToInteger #3427

Closed metasim closed 2 years ago

metasim commented 2 years ago

Details and test case to come, but right now it appears that calls to rows() and cols() box the return values.

:javap output

 public java.lang.Object rows();
    descriptor: ()Ljava/lang/Object;
    flags: (0x1041) ACC_PUBLIC, ACC_BRIDGE, ACC_SYNTHETIC
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: invokevirtual #186                // Method rows:()I
         4: invokestatic  #184                // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
         7: areturn
      LineNumberTable:
        line 146: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       8     0  this   Lgeotrellis/raster/ShortUserDefinedNoDataArrayTile;

  public java.lang.Object cols();
    descriptor: ()Ljava/lang/Object;
    flags: (0x1041) ACC_PUBLIC, ACC_BRIDGE, ACC_SYNTHETIC
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: invokevirtual #178                // Method cols:()I
         4: invokestatic  #184                // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
         7: areturn
      LineNumberTable:
        line 146: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       8     0  this   Lgeotrellis/raster/ShortUserDefinedNoDataArrayTile;

image

pomadchin commented 2 years ago

We did a tiny research with @metasim: RF does a lot of upcasts to the CellGrid[Int] type which leads to ineffective cols() / rows() calls, since Grid and CellGrid are not specialized by Int / Long. In case the Tile is upcasted to it we loose any meta about the specialized version that allows to avoid boxing.

The solution is to add specialization by Int and Long to Grid and / or CellGrid.

Long is needed since the whole idea of this N was to add an ability to work with Longs. (if I don't remember it wrong).

metasim commented 2 years ago

@pomadchin

RF does a lot of upcasts to the CellGrid[Int]

I don't remember these.. are they explicit, and do we need to do anything about them?

pomadchin commented 2 years ago

@metasim nope; I think I meant just general upcasting to parent types with loosing info about specializations in children. Nothing to do about it; should be all good now! Design oversights

pomadchin commented 2 years ago

It is reopened, but the issue is slightly different now: specialization works, however boxing still happens. cc @metasim

metasim commented 2 years ago

Baseline Flight Recording profile.jfr.zip