ARROW-4578: [JS] Ensure Float16 is zero-copy, add more native BigInt support#3653
Closed
trxcllnt wants to merge 16 commits intoapache:masterfrom
Closed
ARROW-4578: [JS] Ensure Float16 is zero-copy, add more native BigInt support#3653trxcllnt wants to merge 16 commits intoapache:masterfrom
trxcllnt wants to merge 16 commits intoapache:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3653 +/- ##
==========================================
+ Coverage 87.47% 90.33% +2.85%
==========================================
Files 627 74 -553
Lines 80413 5413 -75000
Branches 0 1225 +1225
==========================================
- Hits 70344 4890 -65454
+ Misses 9960 515 -9445
+ Partials 109 8 -101
Continue to review full report at Codecov.
|
Member
|
This is great. Native BigInt support will get better and better and so having good support for it in Arrow is great. |
c9613b2 to
7b96b5f
Compare
6350e9e to
2557277
Compare
Member
|
👍 I also appreciate you looking out for firefox users and handling the case where BigInt is not supported Can you rebase now that #3634 is merged? |
… columns by index
…ment extraction util fns
…ible, and accept Iterable<number>
…() and toFloat64Array() methods instead
2557277 to
69ee6f7
Compare
Contributor
Author
|
@TheNeuralBit ok rebase is done. everything looks alright to me, let me know if you notice anything weird though. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This started as a continuation of #3634, but grew enough to deserve its own PR. I've made a PR to my own fork that highlights just the changes here: trxcllnt#8. I'll rebase this PR after #3634 is merged so only these changes are included.
This PR reverts the behavior of
Float16Vector#toArray()back to returning a zero-copy slice of the underlyingUint16Arraydata, and exposes the copying behavior via newtoFloat32Array()andtoFloat64Array()methods.Float16Array.from()will also convert any incoming 32 or 64-bit floats to Uint16s if necessary.It also adds tighter integration with the new
BigInt,BigInt64Array, andBigUint64Arrayprimitives (if available):BigIntto convert/stringify i64s/u64sBigInttype in element comparator andindexOf()toBigInt64Array()andtoBigUint64Array()methods toInt64VectorandUint64Vector, respectively0.4.0 added support for basic conversion to the native
BigIntwhen available, but would only create positiveBigInts, and was slower than necessary. This PR uses the native Arrays to create the BigInts, so we should see some speed ups there. Ex:JIRAs associated with this PR are: