You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/01/01 11:39:22 UTC

[GitHub] [arrow] Dandandan opened a new pull request #9068: ARROW-11100: [Rust] Speed up numeric to string cast using lexical_core

Dandandan opened a new pull request #9068:
URL: https://github.com/apache/arrow/pull/9068


   Uses lexical_core to speed up num to string casts.
   
   ```
   cast i64 to string 512  time:   [22.209 us 22.462 us 22.885 us]
                           change: [-38.438% -37.979% -37.154%] (p = 0.00 < 0.05)
                           Performance has improved.
   Found 14 outliers among 100 measurements (14.00%)
     11 (11.00%) low mild
     3 (3.00%) high severe
   
   Benchmarking cast f32 to string 512: Collecting 100 samples in estimated 5.0698                                                                                 cast f32 to string 512  time:   [25.587 us 25.692 us 25.786 us]
                           change: [-62.364% -62.215% -62.076%] (p = 0.00 < 0.05)
                           Performance has improved.
   Found 7 outliers among 100 measurements (7.00%)
     1 (1.00%) low severe
     1 (1.00%) low mild
     4 (4.00%) high mild
     1 (1.00%) high severe
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] jorgecarleitao commented on a change in pull request #9068: ARROW-11100: [Rust] Speed up numeric to string cast using lexical_core

Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on a change in pull request #9068:
URL: https://github.com/apache/arrow/pull/9068#discussion_r550870918



##########
File path: rust/arrow/src/util/mod.rs
##########
@@ -21,5 +21,6 @@ pub mod display;
 pub mod integration_util;
 #[cfg(feature = "prettyprint")]
 pub mod pretty;
+pub mod serialization;

Review comment:
       what do you think about making this `pub(crate)` or `pub(super)`? In general I think that we should avoid exposing public unless needed, so that it is easier to refactor without breaking public apis.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] codecov-io edited a comment on pull request #9068: ARROW-11100: [Rust] Speed up numeric to string cast using lexical_core

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #9068:
URL: https://github.com/apache/arrow/pull/9068#issuecomment-753307368


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=h1) Report
   > Merging [#9068](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=desc) (48f5522) into [master](https://codecov.io/gh/apache/arrow/commit/cc0ee5efcf9f6a67bcc407a11e8553a0409275c1?el=desc) (cc0ee5e) will **increase** coverage by `0.05%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/9068/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #9068      +/-   ##
   ==========================================
   + Coverage   82.55%   82.61%   +0.05%     
   ==========================================
     Files         203      204       +1     
     Lines       50043    49942     -101     
   ==========================================
   - Hits        41313    41259      -54     
   + Misses       8730     8683      -47     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow/src/compute/kernels/cast.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY29tcHV0ZS9rZXJuZWxzL2Nhc3QucnM=) | `96.99% <100.00%> (ø)` | |
   | [rust/arrow/src/csv/writer.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY3N2L3dyaXRlci5ycw==) | `78.82% <100.00%> (-0.56%)` | :arrow_down: |
   | [rust/arrow/src/util/serialization.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvdXRpbC9zZXJpYWxpemF0aW9uLnJz) | `100.00% <100.00%> (ø)` | |
   | [...datafusion/src/optimizer/hash\_build\_probe\_order.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9kYXRhZnVzaW9uL3NyYy9vcHRpbWl6ZXIvaGFzaF9idWlsZF9wcm9iZV9vcmRlci5ycw==) | `54.73% <0.00%> (-3.70%)` | :arrow_down: |
   | [rust/arrow/src/array/array\_string.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfc3RyaW5nLnJz) | `90.16% <0.00%> (-0.11%)` | :arrow_down: |
   | [rust/datafusion/src/scalar.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9kYXRhZnVzaW9uL3NyYy9zY2FsYXIucnM=) | `58.99% <0.00%> (-0.08%)` | :arrow_down: |
   | [rust/arrow/src/array/array\_binary.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfYmluYXJ5LnJz) | `90.54% <0.00%> (-0.07%)` | :arrow_down: |
   | [rust/parquet/src/arrow/schema.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9zY2hlbWEucnM=) | `90.93% <0.00%> (+0.31%)` | :arrow_up: |
   | [rust/arrow/src/array/array\_primitive.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfcHJpbWl0aXZlLnJz) | `92.32% <0.00%> (+0.68%)` | :arrow_up: |
   | ... and [2 more](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=footer). Last update [cc0ee5e...48f5522](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] Dandandan commented on a change in pull request #9068: ARROW-11100: [Rust] Speed up numeric to string cast using lexical_core

Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #9068:
URL: https://github.com/apache/arrow/pull/9068#discussion_r550871768



##########
File path: rust/arrow/src/util/mod.rs
##########
@@ -21,5 +21,6 @@ pub mod display;
 pub mod integration_util;
 #[cfg(feature = "prettyprint")]
 pub mod pretty;
+pub mod serialization;

Review comment:
       Thanks, makes sense. I think it is indeed best to expose as little as internal code as possible :+1: 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] jorgecarleitao closed pull request #9068: ARROW-11100: [Rust] Speed up numeric to string cast using lexical_core

Posted by GitBox <gi...@apache.org>.
jorgecarleitao closed pull request #9068:
URL: https://github.com/apache/arrow/pull/9068


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] codecov-io edited a comment on pull request #9068: ARROW-11100: [Rust] Speed up numeric to string cast using lexical_core

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #9068:
URL: https://github.com/apache/arrow/pull/9068#issuecomment-753307368


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=h1) Report
   > Merging [#9068](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=desc) (b840784) into [master](https://codecov.io/gh/apache/arrow/commit/cc0ee5efcf9f6a67bcc407a11e8553a0409275c1?el=desc) (cc0ee5e) will **not change** coverage.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/9068/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #9068   +/-   ##
   =======================================
     Coverage   82.55%   82.55%           
   =======================================
     Files         203      204    +1     
     Lines       50043    50043           
   =======================================
     Hits        41313    41313           
     Misses       8730     8730           
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow/src/compute/kernels/cast.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY29tcHV0ZS9rZXJuZWxzL2Nhc3QucnM=) | `96.99% <100.00%> (ø)` | |
   | [rust/arrow/src/csv/writer.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY3N2L3dyaXRlci5ycw==) | `78.82% <100.00%> (-0.56%)` | :arrow_down: |
   | [rust/arrow/src/util/serialization.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvdXRpbC9zZXJpYWxpemF0aW9uLnJz) | `100.00% <100.00%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=footer). Last update [cc0ee5e...b840784](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] codecov-io commented on pull request #9068: ARROW-11100: [Rust] Speed up numeric to string cast using lexical_core

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #9068:
URL: https://github.com/apache/arrow/pull/9068#issuecomment-753307368


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=h1) Report
   > Merging [#9068](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=desc) (05e1901) into [master](https://codecov.io/gh/apache/arrow/commit/cc0ee5efcf9f6a67bcc407a11e8553a0409275c1?el=desc) (cc0ee5e) will **not change** coverage.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/9068/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #9068   +/-   ##
   =======================================
     Coverage   82.55%   82.55%           
   =======================================
     Files         203      204    +1     
     Lines       50043    50043           
   =======================================
     Hits        41313    41313           
     Misses       8730     8730           
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow/src/compute/kernels/cast.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY29tcHV0ZS9rZXJuZWxzL2Nhc3QucnM=) | `96.99% <100.00%> (ø)` | |
   | [rust/arrow/src/csv/writer.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY3N2L3dyaXRlci5ycw==) | `78.82% <100.00%> (-0.56%)` | :arrow_down: |
   | [rust/arrow/src/util/serialization.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvdXRpbC9zZXJpYWxpemF0aW9uLnJz) | `100.00% <100.00%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=footer). Last update [cc0ee5e...b840784](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] github-actions[bot] commented on pull request #9068: ARROW-11100: [Rust] Speed up numeric to string cast using lexical_core

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9068:
URL: https://github.com/apache/arrow/pull/9068#issuecomment-753306954


   https://issues.apache.org/jira/browse/ARROW-11100


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] codecov-io edited a comment on pull request #9068: ARROW-11100: [Rust] Speed up numeric to string cast using lexical_core

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #9068:
URL: https://github.com/apache/arrow/pull/9068#issuecomment-753307368


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=h1) Report
   > Merging [#9068](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=desc) (8929078) into [master](https://codecov.io/gh/apache/arrow/commit/cc0ee5efcf9f6a67bcc407a11e8553a0409275c1?el=desc) (cc0ee5e) will **increase** coverage by `0.05%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/9068/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #9068      +/-   ##
   ==========================================
   + Coverage   82.55%   82.61%   +0.05%     
   ==========================================
     Files         203      204       +1     
     Lines       50043    49942     -101     
   ==========================================
   - Hits        41313    41258      -55     
   + Misses       8730     8684      -46     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow/src/compute/kernels/cast.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY29tcHV0ZS9rZXJuZWxzL2Nhc3QucnM=) | `96.99% <100.00%> (ø)` | |
   | [rust/arrow/src/csv/writer.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY3N2L3dyaXRlci5ycw==) | `78.82% <100.00%> (-0.56%)` | :arrow_down: |
   | [rust/arrow/src/util/serialization.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvdXRpbC9zZXJpYWxpemF0aW9uLnJz) | `100.00% <100.00%> (ø)` | |
   | [...datafusion/src/optimizer/hash\_build\_probe\_order.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9kYXRhZnVzaW9uL3NyYy9vcHRpbWl6ZXIvaGFzaF9idWlsZF9wcm9iZV9vcmRlci5ycw==) | `54.73% <0.00%> (-3.70%)` | :arrow_down: |
   | [rust/parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9lbmNvZGluZ3MvZW5jb2RpbmcucnM=) | `95.24% <0.00%> (-0.20%)` | :arrow_down: |
   | [rust/arrow/src/array/array\_string.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfc3RyaW5nLnJz) | `90.16% <0.00%> (-0.11%)` | :arrow_down: |
   | [rust/datafusion/src/scalar.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9kYXRhZnVzaW9uL3NyYy9zY2FsYXIucnM=) | `58.99% <0.00%> (-0.08%)` | :arrow_down: |
   | [rust/arrow/src/array/array\_binary.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfYmluYXJ5LnJz) | `90.54% <0.00%> (-0.07%)` | :arrow_down: |
   | [rust/parquet/src/arrow/schema.rs](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9zY2hlbWEucnM=) | `90.93% <0.00%> (+0.31%)` | :arrow_up: |
   | ... and [3 more](https://codecov.io/gh/apache/arrow/pull/9068/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=footer). Last update [cc0ee5e...8929078](https://codecov.io/gh/apache/arrow/pull/9068?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org