Skip to content
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

Implement Int64 as a wrapper around int #117

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

osa1
Copy link
Member

@osa1 osa1 commented Oct 17, 2023

This PR adds another Int64 implementation that is just a wrapper around
int, to be used in targets where int is 64 bits.

Becuase dart doc generates using the native class, all documentation comments
are moved to the native class.

The conditional import is implemented based on availability of the library
dart:html, which is only available on dart2js.

(In principle dart2wasm could support dart:html, but it won't be supported,
instead dart2wasm will support package:web)

Fixes #107.

On AOT, JIT, and Wasm we don't need to emulate 64-bit integers as the
`int` type is already 64 bits.

This PR adds another implementation of `Int64` that is just a wrapper
around `int`:

    class Int64 implements IntX {
      final int _i;
      ...
    }

`int64` library imports this implementation in VM and dart2wasm, and the
old (emulated) `Int64` implementation on dart2js.

The conditional import is implemented based on availability of the
library `dart:html`, which is only available on dart2js.

(In principle dart2wasm could support `dart:html` as well, but it won't
be supported, instead dart2wasm will support `package:web`)

Documentations of `Int64` and its members are copied directly from the
old emulated `Int64` class to make sure the documentation will look the
same as before.

The old `toRadixStringUnsigned` implementation is moved to the utilities
library and reused in the new class.
@osa1
Copy link
Member Author

osa1 commented Oct 17, 2023

It looks like analyzer can't handle the conditional import properly, it thinks int64_native and int64_emulated are available at the same time. Does anyone know how to work around this issue?

Locally this passes tests both on Chrome and VM.

@osa1 osa1 requested review from lrhn and mkustermann October 17, 2023 09:35
@osa1 osa1 marked this pull request as ready for review October 17, 2023 09:35
@mkustermann
Copy link
Member

Documentations of Int64 and its members are copied directly ...
It looks like analyzer can't handle the conditional import properly ...

Consider making Int64 an abstract class with factory method that will use a _Int64 class which comes from a conditional import.

Then we wouldn't need to duplicate the comments. The implementation classes may need to do as _Int64 but our compilers should be able to optimize this away (as there's only one implementation class of Int64).

It may also fix the analyzer issue if it mostly operates on Int64 interface (instead of the _Int64 implementation classes)

static Int64? tryParseHex(String source) => _parseRadix(source, 16, false);

static Int64? _parseRadix(String s, int radix, bool throwOnError) {
int charIdx = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can start with

 var tryResult = int.tryParse(s, radix: radix);
 if (tryResult != null) return Int64(tryResult);

That will cover 99% of valid inputs.

(Or we could change the semantics of Int64.parse to not allow overflow, which is silly anyway.)

lib/src/int64_native.dart Outdated Show resolved Hide resolved
lib/src/int64_native.dart Outdated Show resolved Hide resolved
lib/src/int64_native.dart Outdated Show resolved Hide resolved
lib/src/int64_native.dart Outdated Show resolved Hide resolved
lib/src/utilities.dart Outdated Show resolved Hide resolved

@override
List<int> toBytes() {
final result = List<int>.filled(8, 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Really should be returning a Uint8List here. Nobody ever wants anything else. If anyone ever uses this for anything. A writeBytes(List<int> target, int offset) would be more useful.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this should be writeBytes, it makes no sense to serialize an int to a fresh array as you'll always be copying it to another array.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several encodings of numbers that can be written - protobufs use several variable length encodings and it would be most efficient if each of the common encodings had a writeBytes style writer.
The writer would return the new offset.
The matching reader would return a pair (Int64value, newOffset).
Both reader and writer should have a Uint8List argument and probably an end argument and return an offset of -1 if the representation is unterminated before the end.

But I would do nothing in this CL.
Don't change the web toBytes to return an instance of Uint8List. The allocation is prohibitively expensive.

@lrhn
Copy link
Member

lrhn commented Oct 17, 2023

I'd consider moving the package to Dart 3.0 and making the classes final.
But that's a breaking change, and then we might want to consider whether there are more breaking changes we'd want to do at the same time. (Like making toBytes return Uint8List. And not accepting doubles as arguments to operators.)

Heck, we could wait and make the native Int64 be an extension type, for zero memory overhead. (And if we inline the members, also a good chance of very little computational overhead.)

@osa1
Copy link
Member Author

osa1 commented Oct 18, 2023

Heck, we could wait and make the native Int64 be an extension type, for zero memory overhead. (And if we inline the members, also a good chance of very little computational overhead.)

I agree, reported this as #118.

return Int64Impl(i);
}

static int _promote(Object value) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider marking this as

@pragma('vm:prefer-inline')

and equivalent dart2wasm annotation (and similar for the emulated one with dart2js annotation)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's consider it in a separate PR. I want to keep the changes as small as possible to be able to easily test and benchmark, and the current version in master branch doesn't have inline pragmas on this function.

@osa1
Copy link
Member Author

osa1 commented Oct 18, 2023

It looks splitting the class into an abstract + concrete classes doesn't work as well as we thought it would.

In this program:

import 'package:fixnum/fixnum.dart';

List<Int64> fixed64s = [];

void addFixed64(Int64 value) {
  fixed64s.add(value);
}

void main() {
  addFixed64(Int64(123456));
  addFixed64(Int64.MAX_VALUE);
  addFixed64(Int64.MIN_VALUE);

  for (final i in fixed64s) {
    print(i.toRadixStringUnsigned(20));
  }
}

Before this change, when compiled to Wasm, toRadixStringUnsigned is:

(func $Int64.toRadixStringUnsigned (;595;) (param $var0 (ref $Int64)) (param $var1 i64) (result (ref $Object))
  local.get $var1
  call $validateRadix
  local.get $var0
  struct.get $Int64 $field2
  local.get $var0
  struct.get $Int64 $field3
  local.get $var0
  struct.get $Int64 $field4
  global.get $global70
  call $Int64._toRadixStringUnsigned
  return)

After this change:

(func $Int64Impl.toRadixStringUnsigned (;595;) (param $var0 (ref $Object)) (param $var1 i64) (result (ref $Object))
  (local $var2 (ref $Int64Impl))
  local.get $var0
  ref.cast $Int64Impl
  local.set $var2
  local.get $var2
  struct.get $Int64Impl $field2
  local.get $var1
  call $Int64Impl._toRadixStringUnsigned
  return)

The new version takes an Object argument instead of Int64, and downcasts it to Int64Impl.

This is unexpected and there's no reason for this to happen since there is only one implementation class of the type Int64.

I'll try to see if this is a bug in dart2wasm.

@mkustermann do you have any thoughts on what the issue may be?

@rakudrama
Copy link
Member

What kind of code differences do you see for dart2js?
Historically it was very touchy about inferring the fields of the 'emulated' version which need to be inferred as small non-negative integers.

The sdk benchmark sdk/benchmarks/BigIntPrintParse covers radix 10 formatting and parsing.
I'd expect the -O2 JavaScript to be essentially the same modulo a few names.

One concern is that x is Int64 compiles differently - look for _promote and check it uses instanceof in both.

The conditional import is implemented based on availability of the library dart:html, which is only available on dart2js.

dart2js_server (used to compile for nodejs) does not have dart:html (test with dart2js command line argument --server-mode)

@lrhn What we really need here is an import condition that is exactly the feature-detect of web numbers.

(In principle dart2wasm could support dart:html, but it won't be supported, instead dart2wasm will support package:web)

The old toRadixStringUnsigned implementation is moved to the utilities library and reused in the new class.

@osa1
Copy link
Member Author

osa1 commented Oct 19, 2023

dart2wasm issue reported above is fixed in https://dart-review.googlesource.com/c/sdk/+/331102. While investigating this we also discovered some missed optimizations in the TFA, which is reported as dart-lang/sdk#53800.

What kind of code differences do you see for dart2js?

I'll check dart2js outputs of protobuf benchmarks.

@osa1
Copy link
Member Author

osa1 commented Oct 19, 2023

@rakudrama I checked dart2js unoptimized outputs. There's the diff of before vs. after:

dart2js changes in protobuf to_binary after this PR
--- out_before/to_binary.js	2023-10-19 11:24:53.540545406 +0200
+++ out_after/to_binary.js	2023-10-19 11:24:02.564841777 +0200
@@ -3222,7 +3222,7 @@
       this.elapsedMicros = t0;
       this.iterations = t1;
     },
-    Int64_Int64(value) {
+    Int64Impl_Int64Impl(value) {
       var negative, v2, v1, t1, t2, t3;
       if (value < 0) {
         value = -value;
@@ -3235,9 +3235,9 @@
       t1 = value - v1 * 4194304 & 4194303;
       t2 = v1 & 4194303;
       t3 = v2 & 1048575;
-      return negative ? A.Int64__sub(0, 0, 0, t1, t2, t3) : new A.Int64(t1, t2, t3);
+      return negative ? A.Int64Impl__sub(0, 0, 0, t1, t2, t3) : new A.Int64Impl(t1, t2, t3);
     },
-    Int64_Int64$fromBytes(bytes) {
+    Int64Impl_Int64Impl$fromBytes(bytes) {
       var split1, t2, split2, t3, t4,
         t1 = bytes.length;
       if (5 >= t1)
@@ -3250,68 +3250,28 @@
       split2 = bytes[2] & 255;
       t3 = bytes[4];
       t4 = bytes[3];
-      return new A.Int64((split2 << 16 | (bytes[1] & 255) << 8 | bytes[0] & 255) & 4194303, (split1 << 18 | (t3 & 255) << 10 | (t4 & 255) << 2 | split2 >>> 6) & 4194303, ((t1 & 255) << 12 | (t2 & 255) << 4 | split1 >>> 4) & 1048575);
+      return new A.Int64Impl((split2 << 16 | (bytes[1] & 255) << 8 | bytes[0] & 255) & 4194303, (split1 << 18 | (t3 & 255) << 10 | (t4 & 255) << 2 | split2 >>> 6) & 4194303, ((t1 & 255) << 12 | (t2 & 255) << 4 | split1 >>> 4) & 1048575);
     },
-    Int64_Int64$fromInts($top, bottom) {
-      return new A.Int64(bottom & 4194303, (($top & 4095) << 10 | bottom >>> 22 & 1023) & 4194303, $top >>> 12 & 1048575);
+    Int64Impl_Int64Impl$fromInts($top, bottom) {
+      return new A.Int64Impl(bottom & 4194303, (($top & 4095) << 10 | bottom >>> 22 & 1023) & 4194303, $top >>> 12 & 1048575);
     },
-    Int64__promote(value) {
-      if (value instanceof A.Int64)
+    Int64Impl__promote(value) {
+      if (value instanceof A.Int64Impl)
         return value;
       else if (A._isInt(value))
-        return A.Int64_Int64(value);
+        return A.Int64Impl_Int64Impl(value);
       throw A.wrapException(A.ArgumentError$value(value, "other", "not an int, Int32 or Int64"));
     },
-    Int64__toRadixStringUnsigned(radix, d0, d1, d2, sign) {
-      var d4, d3, fatRadix, chunk1, chunk2, chunk3, q, q0, q1, q2, q3, chunk10, residue;
-      if (d0 === 0 && d1 === 0 && d2 === 0)
-        return "0";
-      d4 = (d2 << 4 | d1 >>> 18) >>> 0;
-      d3 = d1 >>> 8 & 1023;
-      d2 = (d1 << 2 | d0 >>> 20) & 1023;
-      d1 = d0 >>> 10 & 1023;
-      d0 &= 1023;
-      if (!(radix < 37))
-        return A.ioore(B.List_Icz, radix);
-      fatRadix = B.List_Icz[radix];
-      chunk1 = "";
-      chunk2 = "";
-      chunk3 = "";
-      while (true) {
-        if (!!(d4 === 0 && d3 === 0))
-          break;
-        q = B.JSInt_methods.$tdiv(d4, fatRadix);
-        d3 += d4 - q * fatRadix << 10 >>> 0;
-        q0 = B.JSInt_methods.$tdiv(d3, fatRadix);
-        d2 += d3 - q0 * fatRadix << 10 >>> 0;
-        q1 = B.JSInt_methods.$tdiv(d2, fatRadix);
-        d1 += d2 - q1 * fatRadix << 10 >>> 0;
-        q2 = B.JSInt_methods.$tdiv(d1, fatRadix);
-        d0 += d1 - q2 * fatRadix << 10 >>> 0;
-        q3 = B.JSInt_methods.$tdiv(d0, fatRadix);
-        chunk10 = B.JSString_methods.substring$1(B.JSInt_methods.toRadixString$1(fatRadix + (d0 - q3 * fatRadix), radix), 1);
-        chunk3 = chunk2;
-        chunk2 = chunk1;
-        chunk1 = chunk10;
-        d3 = q0;
-        d4 = q;
-        d2 = q1;
-        d1 = q2;
-        d0 = q3;
-      }
-      residue = (d2 << 20 >>> 0) + (d1 << 10 >>> 0) + d0;
-      return sign + (residue === 0 ? "" : B.JSInt_methods.toRadixString$1(residue, radix)) + chunk1 + chunk2 + chunk3;
-    },
-    Int64__sub(a0, a1, a2, b0, b1, b2) {
+    Int64Impl__sub(a0, a1, a2, b0, b1, b2) {
       var diff0 = a0 - b0,
         diff1 = a1 - b1 - (B.JSInt_methods._shrOtherPositive$1(diff0, 22) & 1);
-      return new A.Int64(diff0 & 4194303, diff1 & 4194303, a2 - b2 - (B.JSInt_methods._shrOtherPositive$1(diff1, 22) & 1) & 1048575);
+      return new A.Int64Impl(diff0 & 4194303, diff1 & 4194303, a2 - b2 - (B.JSInt_methods._shrOtherPositive$1(diff1, 22) & 1) & 1048575);
     },
-    Int64__shiftRight(x, n) {
+    Int64Impl__shiftRight(x, n) {
       var t1 = B.JSInt_methods._shrReceiverPositive$1(x, n);
       return t1;
     },
-    Int64: function Int64(t0, t1, t2) {
+    Int64Impl: function Int64Impl(t0, t1, t2) {
       this._l = t0;
       this._m = t1;
       this._h = t2;
@@ -3421,7 +3381,7 @@
             break;
           case 16384:
             value = input._readRawVarint64$0();
-            fs._setFieldUnchecked$3(meta, fi, (value.$and(0, 1).$eq(0, 1) ? A.Int64__sub(0, 0, 0, value._l, value._m, value._h) : value).$shr(0, 1));
+            fs._setFieldUnchecked$3(meta, fi, (value.$and(0, 1).$eq(0, 1) ? A.Int64Impl__sub(0, 0, 0, value._l, value._m, value._h) : value).$shr(0, 1));
             break;
           case 32768:
             fs._setFieldUnchecked$3(meta, fi, input._readRawVarint32$1(false));
@@ -3448,7 +3408,7 @@
             t21 = data.buffer;
             t22 = data.byteOffset;
             view = new Uint8Array(t21, t22, 8);
-            fs._setFieldUnchecked$3(meta, fi, A.Int64_Int64$fromBytes(view));
+            fs._setFieldUnchecked$3(meta, fi, A.Int64Impl_Int64Impl$fromBytes(view));
             break;
           case 524288:
             t21 = input._bufferPos += 4;
@@ -3469,7 +3429,7 @@
             t21 = data.buffer;
             t22 = data.byteOffset;
             view = new Uint8Array(t21, t22, 8);
-            fs._setFieldUnchecked$3(meta, fi, A.Int64_Int64$fromBytes(view));
+            fs._setFieldUnchecked$3(meta, fi, A.Int64Impl_Int64Impl$fromBytes(view));
             break;
           case 2097152:
             oldValue = t18._as(fs._getFieldOrNull$1(fi));
@@ -3667,7 +3627,7 @@
         case 65536:
         case 262144:
         case 1048576:
-          if (!(value instanceof A.Int64))
+          if (!(value instanceof A.Int64Impl))
             return "not Int64";
           return _null;
         case 1024:
@@ -4182,6 +4142,46 @@
     },
     throwLateFieldADI(fieldName) {
       A.throwExpressionWithWrapper(new A.LateError("Field '" + fieldName + "' has been assigned during initialization."), new Error());
+    },
+    toRadixStringUnsigned(radix, d0, d1, d2, sign) {
+      var d4, d3, fatRadix, chunk1, chunk2, chunk3, q, q0, q1, q2, q3, chunk10, residue;
+      if (d0 === 0 && d1 === 0 && d2 === 0)
+        return "0";
+      d4 = (d2 << 4 | d1 >>> 18) >>> 0;
+      d3 = d1 >>> 8 & 1023;
+      d2 = (d1 << 2 | d0 >>> 20) & 1023;
+      d1 = d0 >>> 10 & 1023;
+      d0 &= 1023;
+      if (!(radix < 37))
+        return A.ioore(B.List_Icz, radix);
+      fatRadix = B.List_Icz[radix];
+      chunk1 = "";
+      chunk2 = "";
+      chunk3 = "";
+      while (true) {
+        if (!!(d4 === 0 && d3 === 0))
+          break;
+        q = B.JSInt_methods.$tdiv(d4, fatRadix);
+        d3 += d4 - q * fatRadix << 10 >>> 0;
+        q0 = B.JSInt_methods.$tdiv(d3, fatRadix);
+        d2 += d3 - q0 * fatRadix << 10 >>> 0;
+        q1 = B.JSInt_methods.$tdiv(d2, fatRadix);
+        d1 += d2 - q1 * fatRadix << 10 >>> 0;
+        q2 = B.JSInt_methods.$tdiv(d1, fatRadix);
+        d0 += d1 - q2 * fatRadix << 10 >>> 0;
+        q3 = B.JSInt_methods.$tdiv(d0, fatRadix);
+        chunk10 = B.JSString_methods.substring$1(B.JSInt_methods.toRadixString$1(fatRadix + (d0 - q3 * fatRadix), radix), 1);
+        chunk3 = chunk2;
+        chunk2 = chunk1;
+        chunk1 = chunk10;
+        d3 = q0;
+        d4 = q;
+        d2 = q1;
+        d1 = q2;
+        d0 = q3;
+      }
+      residue = (d2 << 20 >>> 0) + (d1 << 10 >>> 0) + d0;
+      return sign + (residue === 0 ? "" : B.JSInt_methods.toRadixString$1(residue, radix)) + chunk1 + chunk2 + chunk3;
     }
   },
   J = {
@@ -5949,15 +5949,15 @@
       return "" + this.elapsedMicros + " in " + this.iterations + " iterations";
     }
   };
-  A.Int64.prototype = {
+  A.Int64Impl.prototype = {
     $and(_, other) {
-      var o = A.Int64__promote(other);
-      return new A.Int64(this._l & o._l & 4194303, this._m & o._m & 4194303, this._h & o._h & 1048575);
+      var o = A.Int64Impl__promote(other);
+      return new A.Int64Impl(this._l & o._l & 4194303, this._m & o._m & 4194303, this._h & o._h & 1048575);
     },
     $shl(_, n) {
       var t1, res0, t2, t3, res1, res2, _this = this;
       if (n >= 64)
-        return B.Int64_0_0_0;
+        return B.Int64Impl_0_0_0;
       if (n < 22) {
         t1 = _this._l;
         res0 = B.JSInt_methods._shlPositive$1(t1, n);
@@ -5977,53 +5977,53 @@
         }
         res0 = 0;
       }
-      return new A.Int64(res0 & 4194303, res1 & 4194303, res2 & 1048575);
+      return new A.Int64Impl(res0 & 4194303, res1 & 4194303, res2 & 1048575);
     },
     $shr(_, n) {
       var a2, negative, res2, t1, t2, res1, res0, _this = this, _1048575 = 1048575, _4194303 = 4194303;
       if (n >= 64)
-        return (_this._h & 524288) !== 0 ? B.Int64_4194303_4194303_1048575 : B.Int64_0_0_0;
+        return (_this._h & 524288) !== 0 ? B.Int64Impl_4194303_4194303_1048575 : B.Int64Impl_0_0_0;
       a2 = _this._h;
       negative = (a2 & 524288) !== 0;
       if (negative && true)
         a2 += 3145728;
       if (n < 22) {
-        res2 = A.Int64__shiftRight(a2, n);
+        res2 = A.Int64Impl__shiftRight(a2, n);
         if (negative)
           res2 |= ~B.JSInt_methods._shrBothPositive$1(_1048575, n) & 1048575;
         t1 = _this._m;
         t2 = 22 - n;
-        res1 = A.Int64__shiftRight(t1, n) | B.JSInt_methods.$shl(a2, t2);
-        res0 = A.Int64__shiftRight(_this._l, n) | B.JSInt_methods.$shl(t1, t2);
+        res1 = A.Int64Impl__shiftRight(t1, n) | B.JSInt_methods.$shl(a2, t2);
+        res0 = A.Int64Impl__shiftRight(_this._l, n) | B.JSInt_methods.$shl(t1, t2);
       } else if (n < 44) {
         res2 = negative ? _1048575 : 0;
         t1 = n - 22;
-        res1 = A.Int64__shiftRight(a2, t1);
+        res1 = A.Int64Impl__shiftRight(a2, t1);
         if (negative)
           res1 |= ~B.JSInt_methods._shrReceiverPositive$1(_4194303, t1) & 4194303;
-        res0 = A.Int64__shiftRight(_this._m, t1) | B.JSInt_methods.$shl(a2, 44 - n);
+        res0 = A.Int64Impl__shiftRight(_this._m, t1) | B.JSInt_methods.$shl(a2, 44 - n);
       } else {
         res2 = negative ? _1048575 : 0;
         res1 = negative ? _4194303 : 0;
         t1 = n - 44;
-        res0 = A.Int64__shiftRight(a2, t1);
+        res0 = A.Int64Impl__shiftRight(a2, t1);
         if (negative)
           res0 |= ~B.JSInt_methods._shrReceiverPositive$1(_4194303, t1) & 4194303;
       }
-      return new A.Int64(res0 & 4194303, res1 & 4194303, res2 & 1048575);
+      return new A.Int64Impl(res0 & 4194303, res1 & 4194303, res2 & 1048575);
     },
     $eq(_, other) {
       var o, _this = this;
       if (other == null)
         return false;
-      if (other instanceof A.Int64)
+      if (other instanceof A.Int64Impl)
         o = other;
       else if (A._isInt(other)) {
         if (_this._h === 0 && _this._m === 0)
           return _this._l === other;
         if ((other & 4194303) === other)
           return false;
-        o = A.Int64_Int64(other);
+        o = A.Int64Impl_Int64Impl(other);
       } else
         o = null;
       if (o != null)
@@ -6034,7 +6034,7 @@
       return this._compareTo$1(other);
     },
     _compareTo$1(other) {
-      var o = A.Int64__promote(other),
+      var o = A.Int64Impl__promote(other),
         t1 = this._h,
         signa = t1 >>> 19,
         t2 = o._h;
@@ -6067,13 +6067,13 @@
       if (width > 64)
         throw A.wrapException(A.RangeError$range(width, 0, 64, null, null));
       if (width > 44)
-        return new A.Int64(_this._l & 4194303, _this._m & 4194303, _this._h & B.JSInt_methods.$shl(1, width - 44) - 1 & 1048575);
+        return new A.Int64Impl(_this._l & 4194303, _this._m & 4194303, _this._h & B.JSInt_methods.$shl(1, width - 44) - 1 & 1048575);
       else {
         t1 = _this._l;
         if (width > 22)
-          return new A.Int64(t1 & 4194303, _this._m & B.JSInt_methods.$shl(1, width - 22) - 1 & 4194303, 0);
+          return new A.Int64Impl(t1 & 4194303, _this._m & B.JSInt_methods.$shl(1, width - 22) - 1 & 4194303, 0);
         else
-          return new A.Int64(t1 & B.JSInt_methods._shlPositive$1(1, width) - 1 & 4194303, 0, 0);
+          return new A.Int64Impl(t1 & B.JSInt_methods._shlPositive$1(1, width) - 1 & 4194303, 0, 0);
       }
     },
     toInt$0(_) {
@@ -6101,9 +6101,10 @@
         sign = "-";
       } else
         sign = "";
-      return A.Int64__toRadixStringUnsigned(10, d0, d1, d2, sign);
+      return A.toRadixStringUnsigned(10, d0, d1, d2, sign);
     },
-    $isComparable: 1
+    $isComparable: 1,
+    $isInt64: 1
   };
   A.BuilderInfo.prototype = {
     add$1$8$protoName(_, tagNumber, $name, fieldType, defaultOrMaker, subBuilder, $valueOf, enumValues, protoName, $T) {
@@ -6153,7 +6154,7 @@
     },
     aInt64$2(tagNumber, $name) {
       var _null = null;
-      this.add$1$8$protoName(0, tagNumber, $name, 4096, B.Int64_0_0_0, _null, _null, _null, _null, type$.Int64);
+      this.add$1$8$protoName(0, tagNumber, $name, 4096, B.Int64Impl_0_0_0, _null, _null, _null, _null, type$.Int64);
     },
     aOB$2(tagNumber, $name) {
       var _null = null;
@@ -6277,11 +6278,11 @@
     },
     readSint64$0() {
       var value = this._readRawVarint64$0(),
-        o = A.Int64__promote(1),
+        o = A.Int64Impl__promote(1),
         t1 = value._l,
         t2 = value._m,
         t3 = value._h;
-      return (new A.Int64(t1 & o._l & 4194303, t2 & o._m & 4194303, t3 & o._h & 1048575).$eq(0, 1) ? A.Int64__sub(0, 0, 0, t1, t2, t3) : value).$shr(0, 1);
+      return (new A.Int64Impl(t1 & o._l & 4194303, t2 & o._m & 4194303, t3 & o._h & 1048575).$eq(0, 1) ? A.Int64Impl__sub(0, 0, 0, t1, t2, t3) : value).$shr(0, 1);
     },
     readFixed32$0() {
       return B.NativeByteData_methods._getUint32$2(this._readByteData$1(4), 0, true);
@@ -6307,7 +6308,7 @@
       split2 = view[2] & 255;
       t3 = view[4];
       t4 = view[3];
-      return new A.Int64((split2 << 16 | (view[1] & 255) << 8 | view[0] & 255) & 4194303, (split1 << 18 | (t3 & 255) << 10 | (t4 & 255) << 2 | split2 >>> 6) & 4194303, ((t1 & 255) << 12 | (t2 & 255) << 4 | split1 >>> 4) & 1048575);
+      return new A.Int64Impl((split2 << 16 | (view[1] & 255) << 8 | view[0] & 255) & 4194303, (split1 << 18 | (t3 & 255) << 10 | (t4 & 255) << 2 | split2 >>> 6) & 4194303, ((t1 & 255) << 12 | (t2 & 255) << 4 | split1 >>> 4) & 1048575);
     },
     readBool$0() {
       return this._readRawVarint32$1(true) !== 0;
@@ -6375,13 +6376,13 @@
         byte = t1[t3];
         lo = (lo | B.JSInt_methods._shlPositive$1(byte & 127, i * 7)) >>> 0;
         if ((byte & 128) === 0)
-          return A.Int64_Int64$fromInts(0, lo);
+          return A.Int64Impl_Int64Impl$fromInts(0, lo);
       }
       byte = _this._readRawVarintByte$0();
       lo = (lo | (byte & 15) << 28) >>> 0;
       hi = byte >>> 4 & 7;
       if ((byte & 128) === 0)
-        return A.Int64_Int64$fromInts(hi, lo);
+        return A.Int64Impl_Int64Impl$fromInts(hi, lo);
       for (i = 0; i < 5; ++i) {
         t3 = ++_this._bufferPos;
         if (t3 > _this._currentLimit)
@@ -6392,7 +6393,7 @@
         byte = t1[t3];
         hi = (hi | B.JSInt_methods._shlPositive$1(byte & 127, i * 7 + 3)) >>> 0;
         if ((byte & 128) === 0)
-          return A.Int64_Int64$fromInts(hi, lo);
+          return A.Int64Impl_Int64Impl$fromInts(hi, lo);
       }
       throw A.wrapException(A.InvalidProtocolBufferException$malformedVarint());
     },
@@ -6682,7 +6683,7 @@
           value.writeToCodedBufferWriter$1(_this);
           break;
         case 2048:
-          _this._writeVarint64$1(A.Int64_Int64(A._asInt(value)));
+          _this._writeVarint64$1(A.Int64Impl_Int64Impl(A._asInt(value)));
           break;
         case 4096:
           _this._writeVarint64$1(type$.Int64._as(value));
@@ -6694,8 +6695,8 @@
         case 16384:
           type$.Int64._as(value);
           t1 = value.$shl(0, 1);
-          o = A.Int64__promote(value.$shr(0, 63));
-          _this._writeVarint64$1(new A.Int64((t1._l ^ o._l) & 4194303, (t1._m ^ o._m) & 4194303, (t1._h ^ o._h) & 1048575));
+          o = A.Int64Impl__promote(value.$shr(0, 63));
+          _this._writeVarint64$1(new A.Int64Impl((t1._l ^ o._l) & 4194303, (t1._m ^ o._m) & 4194303, (t1._h ^ o._h) & 1048575));
           break;
         case 32768:
           _this._writeVarint32$1(A._asInt(value));
@@ -7738,7 +7739,7 @@
       _inherit = hunkHelpers.inherit,
       _inheritMany = hunkHelpers.inheritMany;
     _inherit(A.Object, null);
-    _inheritMany(A.Object, [A.JS_CONST, J.Interceptor, J.ArrayIterator, A.Error, A.Iterable, A.ListIterator, A.MappedIterator, A.FixedLengthListMixin, A.Closure, A.MapBase, A.LinkedHashMapCell, A.LinkedHashMapKeyIterator, A.Rti, A._FunctionParameters, A._Type, A.ListBase, A.Converter, A._Utf8Encoder, A._Utf8Decoder, A.OutOfMemoryError, A.FormatException, A.MapEntry, A.Null, A.Stopwatch, A.StringBuffer, A.BenchmarkBase, A._Measurement, A.Int64, A.BuilderInfo, A.CodedBufferReader, A.CodedBufferWriter, A.InvalidProtocolBufferException, A._ExtensionFieldSet, A._EmptyExtensionRegistry, A.FieldInfo, A._FieldSet, A.GeneratedMessage, A._SingletonMaker, A.PackageName, A.UnknownFieldSet, A.UnknownFieldSetField, A._ResultPrinter]);
+    _inheritMany(A.Object, [A.JS_CONST, J.Interceptor, J.ArrayIterator, A.Error, A.Iterable, A.ListIterator, A.MappedIterator, A.FixedLengthListMixin, A.Closure, A.MapBase, A.LinkedHashMapCell, A.LinkedHashMapKeyIterator, A.Rti, A._FunctionParameters, A._Type, A.ListBase, A.Converter, A._Utf8Encoder, A._Utf8Decoder, A.OutOfMemoryError, A.FormatException, A.MapEntry, A.Null, A.Stopwatch, A.StringBuffer, A.BenchmarkBase, A._Measurement, A.Int64Impl, A.BuilderInfo, A.CodedBufferReader, A.CodedBufferWriter, A.InvalidProtocolBufferException, A._ExtensionFieldSet, A._EmptyExtensionRegistry, A.FieldInfo, A._FieldSet, A.GeneratedMessage, A._SingletonMaker, A.PackageName, A.UnknownFieldSet, A.UnknownFieldSetField, A._ResultPrinter]);
     _inheritMany(J.Interceptor, [J.JSBool, J.JSNull, J.JavaScriptObject, J.JSNumber, J.JSString]);
     _inheritMany(J.JavaScriptObject, [J.LegacyJavaScriptObject, J.JSArray, A.NativeByteBuffer, A.NativeTypedData, A.DomException]);
     _inheritMany(J.LegacyJavaScriptObject, [J.PlainJavaScriptObject, J.UnknownJavaScriptObject, J.JavaScriptFunction]);
@@ -7779,7 +7780,7 @@
     leafTags: null,
     arrayRti: Symbol("$ti")
   };
-  A._Universe_addRules(init.typeUniverse, JSON.parse('{"PlainJavaScriptObject":"LegacyJavaScriptObject","UnknownJavaScriptObject":"LegacyJavaScriptObject","JavaScriptFunction":"LegacyJavaScriptObject","JSBool":{"bool":[],"TrustedGetRuntimeType":[]},"JSNull":{"TrustedGetRuntimeType":[]},"JSArray":{"List":["1"],"EfficientLengthIterable":["1"],"Iterable":["1"],"JSIndexable":["1"]},"JSUnmodifiableArray":{"JSArray":["1"],"List":["1"],"EfficientLengthIterable":["1"],"Iterable":["1"],"JSIndexable":["1"]},"ArrayIterator":{"Iterator":["1"]},"JSNumber":{"double":[],"num":[],"Comparable":["num"]},"JSInt":{"double":[],"int":[],"num":[],"Comparable":["num"],"TrustedGetRuntimeType":[]},"JSNumNotInt":{"double":[],"num":[],"Comparable":["num"],"TrustedGetRuntimeType":[]},"JSString":{"String":[],"Comparable":["String"],"JSIndexable":["@"],"TrustedGetRuntimeType":[]},"EfficientLengthIterable":{"Iterable":["1"]},"ListIterable":{"EfficientLengthIterable":["1"],"Iterable":["1"]},"ListIterator":{"Iterator":["1"]},"MappedIterable":{"Iterable":["2"],"Iterable.E":"2"},"EfficientLengthMappedIterable":{"MappedIterable":["1","2"],"EfficientLengthIterable":["2"],"Iterable":["2"],"Iterable.E":"2"},"MappedIterator":{"Iterator":["2"]},"MappedListIterable":{"ListIterable":["2"],"EfficientLengthIterable":["2"],"Iterable":["2"],"Iterable.E":"2","ListIterable.E":"2"},"Closure":{"Function":[]},"Closure0Args":{"Function":[]},"Closure2Args":{"Function":[]},"TearOffClosure":{"Function":[]},"StaticClosure":{"Function":[]},"BoundClosure":{"Function":[]},"JsLinkedHashMap":{"MapBase":["1","2"],"Map":["1","2"]},"LinkedHashMapKeyIterable":{"EfficientLengthIterable":["1"],"Iterable":["1"],"Iterable.E":"1"},"LinkedHashMapKeyIterator":{"Iterator":["1"]},"NativeByteBuffer":{"TrustedGetRuntimeType":[]},"NativeTypedData":{"TypedData":[]},"NativeByteData":{"ByteData":[],"TypedData":[],"TrustedGetRuntimeType":[]},"NativeTypedArray":{"JavaScriptIndexingBehavior":["1"],"TypedData":[],"JSIndexable":["1"]},"NativeTypedArrayOfInt":{"ListBase":["int"],"JavaScriptIndexingBehavior":["int"],"List":["int"],"EfficientLengthIterable":["int"],"TypedData":[],"JSIndexable":["int"],"Iterable":["int"],"FixedLengthListMixin":["int"]},"NativeUint8List":{"ListBase":["int"],"Uint8List":[],"JavaScriptIndexingBehavior":["int"],"List":["int"],"EfficientLengthIterable":["int"],"TypedData":[],"JSIndexable":["int"],"Iterable":["int"],"FixedLengthListMixin":["int"],"TrustedGetRuntimeType":[],"ListBase.E":"int"},"ListBase":{"List":["1"],"EfficientLengthIterable":["1"],"Iterable":["1"]},"MapBase":{"Map":["1","2"]},"double":{"num":[],"Comparable":["num"]},"int":{"num":[],"Comparable":["num"]},"List":{"EfficientLengthIterable":["1"],"Iterable":["1"]},"num":{"Comparable":["num"]},"String":{"Comparable":["String"]},"Int64":{"Comparable":["Object"]},"Extension":{"FieldInfo":["1"]},"_EmptyExtensionRegistry":{"ExtensionRegistry":[]},"PbList":{"ListBase":["1"],"List":["1"],"EfficientLengthIterable":["1"],"Iterable":["1"],"ListBase.E":"1"},"_ResultPrinter":{"ScoreEmitter":[]},"GoogleMessage10":{"GeneratedMessage":[]},"GoogleMessage1SubMessage0":{"GeneratedMessage":[]},"GoogleMessage1":{"GeneratedMessage":[]},"GoogleMessage1SubMessage":{"GeneratedMessage":[]},"GoogleMessage2_Group1":{"GeneratedMessage":[]},"GoogleMessage2":{"GeneratedMessage":[]},"GoogleMessage2GroupedMessage":{"GeneratedMessage":[]},"ByteData":{"TypedData":[]},"Uint8List":{"List":["int"],"EfficientLengthIterable":["int"],"Iterable":["int"],"TypedData":[]}}'));
+  A._Universe_addRules(init.typeUniverse, JSON.parse('{"PlainJavaScriptObject":"LegacyJavaScriptObject","UnknownJavaScriptObject":"LegacyJavaScriptObject","JavaScriptFunction":"LegacyJavaScriptObject","JSBool":{"bool":[],"TrustedGetRuntimeType":[]},"JSNull":{"TrustedGetRuntimeType":[]},"JSArray":{"List":["1"],"EfficientLengthIterable":["1"],"Iterable":["1"],"JSIndexable":["1"]},"JSUnmodifiableArray":{"JSArray":["1"],"List":["1"],"EfficientLengthIterable":["1"],"Iterable":["1"],"JSIndexable":["1"]},"ArrayIterator":{"Iterator":["1"]},"JSNumber":{"double":[],"num":[],"Comparable":["num"]},"JSInt":{"double":[],"int":[],"num":[],"Comparable":["num"],"TrustedGetRuntimeType":[]},"JSNumNotInt":{"double":[],"num":[],"Comparable":["num"],"TrustedGetRuntimeType":[]},"JSString":{"String":[],"Comparable":["String"],"JSIndexable":["@"],"TrustedGetRuntimeType":[]},"EfficientLengthIterable":{"Iterable":["1"]},"ListIterable":{"EfficientLengthIterable":["1"],"Iterable":["1"]},"ListIterator":{"Iterator":["1"]},"MappedIterable":{"Iterable":["2"],"Iterable.E":"2"},"EfficientLengthMappedIterable":{"MappedIterable":["1","2"],"EfficientLengthIterable":["2"],"Iterable":["2"],"Iterable.E":"2"},"MappedIterator":{"Iterator":["2"]},"MappedListIterable":{"ListIterable":["2"],"EfficientLengthIterable":["2"],"Iterable":["2"],"Iterable.E":"2","ListIterable.E":"2"},"Closure":{"Function":[]},"Closure0Args":{"Function":[]},"Closure2Args":{"Function":[]},"TearOffClosure":{"Function":[]},"StaticClosure":{"Function":[]},"BoundClosure":{"Function":[]},"JsLinkedHashMap":{"MapBase":["1","2"],"Map":["1","2"]},"LinkedHashMapKeyIterable":{"EfficientLengthIterable":["1"],"Iterable":["1"],"Iterable.E":"1"},"LinkedHashMapKeyIterator":{"Iterator":["1"]},"NativeByteBuffer":{"TrustedGetRuntimeType":[]},"NativeTypedData":{"TypedData":[]},"NativeByteData":{"ByteData":[],"TypedData":[],"TrustedGetRuntimeType":[]},"NativeTypedArray":{"JavaScriptIndexingBehavior":["1"],"TypedData":[],"JSIndexable":["1"]},"NativeTypedArrayOfInt":{"ListBase":["int"],"JavaScriptIndexingBehavior":["int"],"List":["int"],"EfficientLengthIterable":["int"],"TypedData":[],"JSIndexable":["int"],"Iterable":["int"],"FixedLengthListMixin":["int"]},"NativeUint8List":{"ListBase":["int"],"Uint8List":[],"JavaScriptIndexingBehavior":["int"],"List":["int"],"EfficientLengthIterable":["int"],"TypedData":[],"JSIndexable":["int"],"Iterable":["int"],"FixedLengthListMixin":["int"],"TrustedGetRuntimeType":[],"ListBase.E":"int"},"ListBase":{"List":["1"],"EfficientLengthIterable":["1"],"Iterable":["1"]},"MapBase":{"Map":["1","2"]},"double":{"num":[],"Comparable":["num"]},"int":{"num":[],"Comparable":["num"]},"List":{"EfficientLengthIterable":["1"],"Iterable":["1"]},"num":{"Comparable":["num"]},"String":{"Comparable":["String"]},"Int64Impl":{"Int64":[],"Comparable":["Object"]},"Extension":{"FieldInfo":["1"]},"_EmptyExtensionRegistry":{"ExtensionRegistry":[]},"PbList":{"ListBase":["1"],"List":["1"],"EfficientLengthIterable":["1"],"Iterable":["1"],"ListBase.E":"1"},"_ResultPrinter":{"ScoreEmitter":[]},"GoogleMessage10":{"GeneratedMessage":[]},"GoogleMessage1SubMessage0":{"GeneratedMessage":[]},"GoogleMessage1":{"GeneratedMessage":[]},"GoogleMessage1SubMessage":{"GeneratedMessage":[]},"GoogleMessage2_Group1":{"GeneratedMessage":[]},"GoogleMessage2":{"GeneratedMessage":[]},"GoogleMessage2GroupedMessage":{"GeneratedMessage":[]},"ByteData":{"TypedData":[]},"Uint8List":{"List":["int"],"EfficientLengthIterable":["int"],"Iterable":["int"],"TypedData":[]},"Int64":{"Comparable":["Object"]}}'));
   A._Universe_addErasedTypes(init.typeUniverse, JSON.parse('{"EfficientLengthIterable":1,"NativeTypedArray":1,"Converter":2}'));
   var string$ = {
     CodedB: "CodedBufferReader encountered an embedded string or message which claimed to have negative size."
@@ -7982,8 +7983,8 @@
     B.C_Utf8Encoder = new A.Utf8Encoder();
     B.C__EmptyExtensionRegistry = new A._EmptyExtensionRegistry();
     B.C__ResultPrinter = new A._ResultPrinter();
-    B.Int64_0_0_0 = new A.Int64(0, 0, 0);
-    B.Int64_4194303_4194303_1048575 = new A.Int64(4194303, 4194303, 1048575);
+    B.Int64Impl_0_0_0 = new A.Int64Impl(0, 0, 0);
+    B.Int64Impl_4194303_4194303_1048575 = new A.Int64Impl(4194303, 4194303, 1048575);
     B.List_Icz = A._setArrayType(makeConstList([0, 0, 1048576, 531441, 1048576, 390625, 279936, 823543, 262144, 531441, 1000000, 161051, 248832, 371293, 537824, 759375, 1048576, 83521, 104976, 130321, 160000, 194481, 234256, 279841, 331776, 390625, 456976, 531441, 614656, 707281, 810000, 923521, 1048576, 35937, 39304, 42875, 46656]), type$.JSArray_int);
     B.PackageName_Mno = new A.PackageName("");
     B.PackageName_QTL = new A.PackageName("benchmarks.proto2");
@@ -8111,7 +8112,7 @@
       t1.a$1$4$defaultOrMaker(19, "field19", _2048, 2, t2);
       t1.a$1$4$defaultOrMaker(20, "field20", 16, true, t3);
       t4 = type$.Int64;
-      t1.a$1$4$defaultOrMaker(21, "field21", 262144, B.Int64_0_0_0, t4);
+      t1.a$1$4$defaultOrMaker(21, "field21", 262144, B.Int64Impl_0_0_0, t4);
       t1.a$1$3(22, "field22", _2048, t2);
       t1.aOB$2(23, "field23");
       t1.a$1$4$defaultOrMaker(28, "field28", 16, true, t3);
@@ -8119,8 +8120,8 @@
       t1.a$1$3(204, "field204", _2048, t2);
       t1.aOS$2(205, "field205");
       t1.aOB$2(206, "field206");
-      t1.a$1$4$defaultOrMaker(207, "field207", 65536, B.Int64_0_0_0, t4);
-      t1.a$1$4$defaultOrMaker(300, "field300", 65536, B.Int64_0_0_0, t4);
+      t1.a$1$4$defaultOrMaker(207, "field207", 65536, B.Int64Impl_0_0_0, t4);
+      t1.a$1$4$defaultOrMaker(300, "field300", 65536, B.Int64Impl_0_0_0, t4);
       return t1;
     });
     _lazyFinal($, "GoogleMessage1__i0", "$get$GoogleMessage1__i", () => {
@@ -8185,7 +8186,7 @@
       t1.a$1$3(19, "field19", _2048, t2);
       t1.aOB$2(20, "field20");
       t3 = type$.Int64;
-      t1.a$1$4$defaultOrMaker(21, "field21", 262144, B.Int64_0_0_0, t3);
+      t1.a$1$4$defaultOrMaker(21, "field21", 262144, B.Int64Impl_0_0_0, t3);
       t1.a$1$3(22, "field22", _2048, t2);
       t1.aOB$2(23, "field23");
       t1.aOB$2(28, "field28");
@@ -8193,8 +8194,8 @@
       t1.a$1$3(204, "field204", _2048, t2);
       t1.aOS$2(205, "field205");
       t1.aOB$2(206, "field206");
-      t1.a$1$4$defaultOrMaker(207, "field207", 65536, B.Int64_0_0_0, t3);
-      t1.a$1$4$defaultOrMaker(300, "field300", 65536, B.Int64_0_0_0, t3);
+      t1.a$1$4$defaultOrMaker(207, "field207", 65536, B.Int64Impl_0_0_0, t3);
+      t1.a$1$4$defaultOrMaker(300, "field300", 65536, B.Int64Impl_0_0_0, t3);
       return t1;
     });
     _lazyFinal($, "GoogleMessage2_Group1__i", "$get$GoogleMessage2_Group1__i", () => {
@@ -8207,7 +8208,7 @@
       t1.aOS$2(12, "field12");
       t1.aOS$2(13, "field13");
       t1.pPS$2(14, "field14");
-      t1.a$1$4$defaultOrMaker(15, "field15", 65537, B.Int64_0_0_0, type$.Int64);
+      t1.a$1$4$defaultOrMaker(15, "field15", 65537, B.Int64Impl_0_0_0, type$.Int64);
       t1.aOS$2(16, "field16");
       t1.a$1$3(20, "field20", 2048, t2);
       t1.pPS$2(22, "field22");

There are two changes:

  • Naming changes caused by the new implementation class named Int64Impl (instead of Int64).
  • Because the static member toRadixStringUnsigned moved from the class to the utilities module, it's location in the generated .js file and name changed.

So it looks fine to me.

@osa1
Copy link
Member Author

osa1 commented Oct 19, 2023

One concern is that x is Int64 compiles differently - look for _promote and check it uses instanceof in both.

Before:

Int64__promote(value) {
  if (value instanceof A.Int64)
    return value;
  else if (A._isInt(value))
    return A.Int64_Int64(value);
  throw A.wrapException(A.ArgumentError$value(value, "other", "not an int, Int32 or Int64"));
},

After:

Int64Impl__promote(value) {
  if (value instanceof A.Int64Impl)
    return value;
  else if (A._isInt(value))
    return A.Int64Impl_Int64Impl(value);
  throw A.wrapException(A.ArgumentError$value(value, "other", "not an int, Int32 or Int64"));
}


@override
List<int> toBytes() {
final result = List<int>.filled(8, 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several encodings of numbers that can be written - protobufs use several variable length encodings and it would be most efficient if each of the common encodings had a writeBytes style writer.
The writer would return the new offset.
The matching reader would return a pair (Int64value, newOffset).
Both reader and writer should have a Uint8List argument and probably an end argument and return an offset of -1 if the representation is unterminated before the end.

But I would do nothing in this CL.
Don't change the web toBytes to return an instance of Uint8List. The allocation is prohibitively expensive.

lib/src/int64_emulated.dart Outdated Show resolved Hide resolved
lib/src/int64_native.dart Outdated Show resolved Hide resolved
lib/src/utilities.dart Outdated Show resolved Hide resolved
…efficiently in the native class

Co-authored-by: Stephen Adams <sra@google.com>
@osa1
Copy link
Member Author

osa1 commented Oct 20, 2023

Blocked on dart-lang/sdk#53811.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better representation of Int64 on platforms that support unboxed int64 integers
4 participants