search

wstrange / asn1lib

Dart ASN1 Encoder / Decoder
BSD 2-Clause "Simplified" License
30 stars 30 forks source link

Remove int64list #20

Closed pulyaevskiy closed 8 years ago

pulyaevskiy commented 8 years ago

Hey,

This is an attempt to remove dependency on Int64List and replace it with BigInteger. It seems that there is everything needed already and I'm not sure why original implementation used Int64List.

In any case, tests are passing.

Addresses #18 .

stevenroose commented 8 years ago

I think the original rationale may have been that Int64List is a core package while the bignum package is a port of some JS implementation. It works, but the code is really dirty. And even though bignum is optimized for dart2js use, it is not necessary better when using the Dart VM. (I have not seen any benchmarks on this, though.)

stevenroose commented 8 years ago

Tests run successfully for me in the VM. Sadly testing in the browser using pub run test --platform "browser" is not possible due to the use of the .pem files.

stevenroose commented 8 years ago

Otherwise seems fine, so thanks!

pulyaevskiy commented 8 years ago

I see, so the "ideal" solution in this case would be to replace bignum completely with dart:typed_data (avoiding Int64)?

stevenroose commented 8 years ago

@pulyaevskiy Int64List is a class from typed_data... But there is no implementation for integers of infinite size.

Another big integer implementation is the one used by the rational and decimal packages: https://github.com/a14n/dart-rational/blob/master/lib/bigint.dart

The implementation is a lot cleaner, but I suspect it to be less performant when compiled to JS, because the current bignum implementation is taken from an optimized JS implementation.