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

js: use BigInt64Array or BigInt instead of Number for int64, int, uint64, uint #187

Closed
timotheecour opened this issue Jan 29, 2020 · 11 comments · Fixed by nim-lang/Nim#21613

Comments

@timotheecour
Copy link
Member

timotheecour commented Jan 29, 2020

in nim js, int (and even int64 etc) is represented as Number, which can hold 32 bit integers but behave weirdly beyond that (eg float). As mentioned here [1]:

Large integer IDs and high-accuracy timestamps cannot safely be represented as Numbers in JavaScript. This often leads to real-world bugs, and causes JavaScript developers to represent them as strings instead. With BigInt, this data can now be represented as numeric values.

BigInt is supported not everywhere but in most major browsers: https://caniuse.com/#search=bigint ; and you can easily play with it, eg in chrome console:

BigInt(Number.MAX_SAFE_INTEGER) + 2n; // works
Number.MAX_SAFE_INTEGER + 2; // would not work, showing limitations of Number

it's part of ES2020 https://www.freecodecamp.org/news/javascript-new-features-es2020/

proposal

--experimental:js_enable_bigint that makes int, int64, uint64 use BigInt instead of Number for nim js

links

@kristianmandrup
Copy link

Looking into jsgen.nim in Nim, I find the following:

of mUnaryMinusI64: applyFormat("negInt64($1)", "-($1)")
# ...
  of mInt64ToStr: applyFormat("cstrToNimstr(($1)+\"\")", "cstrToNimstr(($1)+\"\")")

and in proc gen

  of nkCharLit..nkUInt64Lit:
    if n.typ.kind == tyBool:
      r.res = if n.intVal == 0: rope"false" else: rope"true"
    else:
      r.res = rope(n.intVal)

I would think it the key is to target Int64Lit?

@timotheecour
Copy link
Member Author

take a look at nim-lang/Nim#13298 as a starting point; I'll let you take over :-)

@metagn
Copy link
Contributor

metagn commented Apr 26, 2020

nim-lang/Nim#4714 (comment) links to Scala JS implementation of Long, and that is with 2 seperate 32 bit integers, someone also linked their repo with a port of the Kotlin solution. Since Nim targets Javascript 1.5 (or thats what the backends page says) this might be a better solution.

@timotheecour
Copy link
Member Author

definitely worth considering (analog to compiler/int128.nim), but I suspect it will be slower than BigInt. We should run some benchmarks.
Supporting js 1.5 is fine (either as option or default), but that shouldn't be imposed either for client code that cares about performance and doesn't need to target 1.5; so that approach could be a fallback, at least. And there are other useful new features too.

@timotheecour
Copy link
Member Author

timotheecour commented Oct 5, 2020

just found this relevant article: https://medium.com/leaningtech/how-cheerp-supports-64-bit-integers-in-both-javascript-and-webassembly-79485761615a for how the handle 64 bit integers in js, exploring bigint, wasm, and a custom solution.

  • Cheerp is a C/C++ => js an (open source dual licensed) compiler that could have some useful ideas for nim's js target.
  • Cheerpx is another product that seems impressive (but not yet release IIUC), running x86 binaries and OS's purely client side

@metagn
Copy link
Contributor

metagn commented Oct 5, 2020

Although that article expresses problems with BigInt, double int32 is not much better for our pure JS output. BigInt is slow and supported less and incompatible with the Number type but has convenient things like asIntN and BigInt64Array. Double int32 entirely clashes with the handling of all the other numbers and we would have to choose between an object and an array.

It's not going to be fun either way. Both things could be done in a library/module, maybe that could be done before implementing it in the core language to better gauge the semantics.

@timotheecour timotheecour changed the title js: use BigInt instead of Number for int64, int js: use BigInt instead of Number for int64, int, uint64, uint Dec 21, 2020
@timotheecour
Copy link
Member Author

there's also the more recent BigInt64Array and BigUInt64Array, refs https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt64Array

The BigInt64Array typed array represents an array of 64-bit signed integers in the platform byte order

it's supported on almost all browsers except safari (and IE), for now

@timotheecour timotheecour changed the title js: use BigInt instead of Number for int64, int, uint64, uint js: use BigInt64Array or BigInt instead of Number for int64, int, uint64, uint Jan 27, 2021
@Clyybber
Copy link

Clyybber commented Jan 27, 2021

It's supported on less than 3/4ths of the browsers currently in use: https://caniuse.com/mdn-javascript_builtins_bigint64array
I did not find any polyfills for them; GoogleChromeLabs/jsbi#4

@ringabout
Copy link
Member

Safari seems to begin to implement bigInt64Array and BigUInt64Array
https://bugs.webkit.org/show_bug.cgi?id=190800

@timotheecour
Copy link
Member Author

https://bugs.webkit.org/show_bug.cgi?id=190800 is now marked as fixed

@metagn
Copy link
Contributor

metagn commented Apr 4, 2023

IMO we should use BigInt.

  1. BigInt64Array or a library solution will have reference semantics which the codegen would have to handle differently than all the other integer types unless we copy it all the time which feels like it would be terrible performance and would still be handled differently
  2. The fact that BigInt64Array exists and uses bigints for the API implies to me that VMs might have the ability to start off bigints as 64 bit integers then change to an allocated bigint as needed (like Python?), meaning it could have the best performance compared to any library solution or BigInt64Array
  3. It's compatible with using BigInt64Array for openarray[int64]
  4. Just in general BigInt would probably have the simplest codegen out of any other option by far

It's weird to me that no one has attempted a PR after nim-lang/Nim#13298 unless I'm missing something.

Also I don't know why int and uint are mentioned both here and in nim-lang/Nim#4714. There's no reason for them to not stay as they are.

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 a pull request may close this issue.

5 participants