scala-native icon indicating copy to clipboard operation
scala-native copied to clipboard

posix time_t as an object forces C Interop slowdown

Open LeeTibbert opened this issue 2 years ago • 6 comments

It appears that after the recent merge of 32 bit support that posix time_t (and many others) is now an object (AnyRef, if I have my terminology right) rather than an Integer (AnyVal).

This means that data structures which could, with appropriate guards to ensure compataiblity, that previously could be passed to and from C now require a layer of "glue" to manually copy fields between now grossly different Scala Native constructs and the C/rust/mumble variant. I shudder to think of the execution time slowdown.

In my immediate case, clock_gettime() now silently returns garbage. Yes, I can fix that, given time, but I wonder how many other cases of silent garbage need to be sorted out.

LeeTibbert avatar Jul 15 '22 02:07 LeeTibbert

@shadaj First of all, congratulations on getting 32 bit support merged. I believe that you have been working on it for years.

Is my reading of the new Size construct correct, that it is a compound object, not just 32 or 64 bits. Is the compiler doing some magic behind the scene?

Are there other "significant" changes that other Scala Native contributors might like to know before they run into them? A "Gentle Guide to 32 bit Scala Native".

I have already tripped over a few things like now having to do an explicit .toInt for comparisons of a value of type Size to Integers, such as 0 or -1. Compiler started complaining of those on previously working code and I had to suss them out.

LeeTibbert avatar Jul 15 '22 02:07 LeeTibbert

I probably betray myself by speaking/writing to be a barbarian and one who does not appreciate the elegance of the presented solutions.

I think that what I would ask for is to use the link time resolution to allow time_t (and many others in time.scala, socket.scala, netdb.scala,) to be scalar integers, 32 or 64 bit as the case may be. Anybody familiar with those structures is going to be Astonished! at them containing AnyRefs.

Yes, Life is change bit it can also be the easy side of hard.

LeeTibbert avatar Jul 15 '22 02:07 LeeTibbert

Not at my laptop, so a shorter comment for now. I'm surprised that you are seeing AnyRefs. Size/USize are handled specially by the compiler to be treated as primitives at the interop layer (otherwise all of Scala Native would be completely broken). They are meant to be similar to the unsigned integer types, which are defined as classes but handled specially so that they don't box and can be used for interop. Something funny must be going on if you are seeing garbage data when using the Size type.

shadaj avatar Jul 15 '22 03:07 shadaj

@shadaj Thank you for the timely reply. I said AnyRef because Size declaration looks like an object.

Perhaps, when you have a chance, a walk through of some of the compiler magic for those of us in the cheap seats.

My concern is specifically the timespec declarations in posixlib time.scala & time.c. If they are still OK, I think we need to drop some comments in briefly explaining the magic (or a reference to someplace in the Contributor's Guide, so that the explanation can be done in one place, not 100 files). If they are hosed, could you give some guidance for recovery?

I have a clock_gettime PR which I just moved to the sideline because it looks like the timespec filled in by the OS would differ from any Scala Native timespec.

I took a run at trying to define some "glue" in time.c with a `scalanative_timespec' and boggled at the time_t argument (if it is now an object).

Thank you for your help.

LeeTibbert avatar Jul 15 '22 03:07 LeeTibbert

I've made a quick comparison between simple use case of transition that happened here in terms of changes to Size type based on the following snippet. It's not the same case as in your issue, but I had no time yet to create a proper comparison that would also not involve any additional side effects and boxing, hower it should show how introduced changes might change generated code. The output is compared after the optimizer run in the debug mode. I would try to create a similar comparison for interaction with externs soon.

object Test {
  def main(args: Array[String]): Unit = {
    import scalanative.unsafe._
    import scalanative.unsigned._
    val x = sizeof[String]
    scalanative.runtime.Intrinsics.stackalloc(x)

    val y = ssizeof[String]
    scalanative.runtime.Intrinsics.stackalloc(y.toUSize)
  }
}

--- out-opt-0.4	2022-07-15 15:19:34.833328309 +0000
+++ out-opt-0.5	2022-07-15 15:25:11.389030099 +0000
@@ -10,12 +10,10 @@
   %50001 = classalloc @"T34scala.scalanative.unsafe.Tag$Class"
   %50002 = fieldstore[@"T15java.lang.Class"] %50001 : !?@"T34scala.scalanative.unsafe.Tag$Class", @"M34scala.scalanative.unsafe.Tag$ClassF2ofPT34scala.scalanative.unsafe.Tag$Class", %40008 : @"T15java.lang.Class"
   %50003 = call[(@"T13scala.Product") => unit] @"M13scala.ProductD6$init$uEO" : ptr(%50001 : !?@"T34scala.scalanative.unsafe.Tag$Class")
-  %90001 = module @"T51scala.scalanative.unsigned.package$UnsignedRichInt$"
-  %90002 = module @"T35scala.scalanative.unsigned.package$"
-  %90003 = call[(@"T35scala.scalanative.unsigned.package$", int) => int] @"M35scala.scalanative.unsigned.package$D15UnsignedRichIntiiEO" : ptr(%90002 : !?@"T35scala.scalanative.unsigned.package$", int 8)
-  %90004 = call[(@"T51scala.scalanative.unsigned.package$UnsignedRichInt$", int) => @"T32scala.scalanative.unsigned.ULong"] @"M51scala.scalanative.unsigned.package$UnsignedRichInt$D17toULong$extensioniL32scala.scalanative.unsigned.ULongEO" : ptr(%90001 : !?@"T51scala.scalanative.unsigned.package$UnsignedRichInt$", %90003 : int)
-  %30005 = unbox[@"T32scala.scalanative.unsigned.ULong"] %90004 : @"T32scala.scalanative.unsigned.ULong"
-  %30006 = stackalloc[byte] %30005 : long
+  %90001 = module @"T33scala.scalanative.unsafe.TagUtil$"
+  %100001 = fieldload[@"T32scala.scalanative.unsigned.USize"] %90001 : !?@"T33scala.scalanative.unsafe.TagUtil$", @"M33scala.scalanative.unsafe.TagUtil$F7ptrSizePT33scala.scalanative.unsafe.TagUtil$"
+  %30005 = unbox[@"T32scala.scalanative.unsigned.USize"] %100001 : @"T32scala.scalanative.unsigned.USize"
+  %30006 = stackalloc[byte] %30005 : size
   %30007 = call[(@"T23scala.reflect.ClassTag$", @"T15java.lang.Class") => @"T22scala.reflect.ClassTag"] @"M23scala.reflect.ClassTag$D5applyL15java.lang.ClassL22scala.reflect.ClassTagEO" : ptr(%30003 : !?@"T23scala.reflect.ClassTag$", classOf[@"T16java.lang.String"])
   %100003 = call[(@"T13scala.Predef$", @"T16java.lang.Object") => @"T16java.lang.Object"] @"M13scala.Predef$D10implicitlyL16java.lang.ObjectL16java.lang.ObjectEO" : ptr(%40003 : !?@"T13scala.Predef$", %30007 : @"T22scala.reflect.ClassTag")
   %100004 = as[@"T22scala.reflect.ClassTag"] %100003 : @"T16java.lang.Object"
@@ -24,13 +22,10 @@
   %110001 = classalloc @"T34scala.scalanative.unsafe.Tag$Class"
   %110002 = fieldstore[@"T15java.lang.Class"] %110001 : !?@"T34scala.scalanative.unsafe.Tag$Class", @"M34scala.scalanative.unsafe.Tag$ClassF2ofPT34scala.scalanative.unsafe.Tag$Class", %100007 : @"T15java.lang.Class"
   %110003 = call[(@"T13scala.Product") => unit] @"M13scala.ProductD6$init$uEO" : ptr(%110001 : !?@"T34scala.scalanative.unsafe.Tag$Class")
-  %150001 = call[(@"T35scala.scalanative.unsigned.package$", int) => int] @"M35scala.scalanative.unsigned.package$D15UnsignedRichIntiiEO" : ptr(%90002 : !?@"T35scala.scalanative.unsigned.package$", int 8)
-  %150002 = call[(@"T51scala.scalanative.unsigned.package$UnsignedRichInt$", int) => @"T32scala.scalanative.unsigned.ULong"] @"M51scala.scalanative.unsigned.package$UnsignedRichInt$D17toULong$extensioniL32scala.scalanative.unsigned.ULongEO" : ptr(%90001 : !?@"T51scala.scalanative.unsigned.package$UnsignedRichInt$", %150001 : int)
-  %140001 = call[(@"T32scala.scalanative.unsigned.ULong") => long] @"M32scala.scalanative.unsigned.ULongD6toLongjEO" : ptr(%150002 : @"T32scala.scalanative.unsigned.ULong")
-  %30008 = module @"T52scala.scalanative.unsigned.package$UnsignedRichLong$"
-  %30009 = call[(@"T35scala.scalanative.unsigned.package$", long) => long] @"M35scala.scalanative.unsigned.package$D16UnsignedRichLongjjEO" : ptr(%90002 : !?@"T35scala.scalanative.unsigned.package$", %140001 : long)
-  %30010 = call[(@"T52scala.scalanative.unsigned.package$UnsignedRichLong$", long) => @"T32scala.scalanative.unsigned.ULong"] @"M52scala.scalanative.unsigned.package$UnsignedRichLong$D17toULong$extensionjL32scala.scalanative.unsigned.ULongEO" : ptr(%30008 : !?@"T52scala.scalanative.unsigned.package$UnsignedRichLong$", %30009 : long)
-  %30011 = unbox[@"T32scala.scalanative.unsigned.ULong"] %30010 : @"T32scala.scalanative.unsigned.ULong"
-  %30012 = stackalloc[byte] %30011 : long
+  %170001 = fieldload[@"T32scala.scalanative.unsigned.USize"] %90001 : !?@"T33scala.scalanative.unsafe.TagUtil$", @"M33scala.scalanative.unsafe.TagUtil$F7ptrSizePT33scala.scalanative.unsafe.TagUtil$"
+  %150001 = call[(@"T32scala.scalanative.unsigned.USize") => @"T29scala.scalanative.unsafe.Size"] @"M32scala.scalanative.unsigned.USizeD6toSizeL29scala.scalanative.unsafe.SizeEO" : ptr(%170001 : @"T32scala.scalanative.unsigned.USize")
+  %30008 = call[(@"T29scala.scalanative.unsafe.Size") => @"T32scala.scalanative.unsigned.USize"] @"M29scala.scalanative.unsafe.SizeD7toUSizeL32scala.scalanative.unsigned.USizeEO" : ptr(%150001 : @"T29scala.scalanative.unsafe.Size")
+  %30009 = unbox[@"T32scala.scalanative.unsigned.USize"] %30008 : @"T32scala.scalanative.unsigned.USize"
+  %30010 = stackalloc[byte] %30009 : size
   ret unit
 }

WojciechMazur avatar Jul 15 '22 15:07 WojciechMazur

@WojciechMazur Thank you.

Folks,

By the light of day I have done some tests both with clock_gettime and some of the socket code. I think the issue is "It looks like a platypus but is really walks and quacks like the duck we all love & expect".

In plain English, the declarations look like references to objects (AnyRef) but, modulo some newly required toSize & toInt conversions) presents to and from C as a 32 or 64 bit actual value (I will not say real to avoid confusion with floating point).

`clock_gettime() is returning to Scala Native structures values consistent with a straight C program.

So, I think what is missing here is a description some where that "Size looks gnarly but due to compiler magic it is really OK (were OK is better described as what C (or other InterOp) sees and returns."

LeeTibbert avatar Jul 15 '22 16:07 LeeTibbert