ARROW-9453: [Rust] Wasm32 compilation support #7767

ghost · 2020-07-15T00:15:46Z

Adding compilation flag to allow rust library to compile against Wasm32 target

github-actions · 2020-07-15T00:41:26Z

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

paddyhoran · 2020-07-15T02:39:39Z

rust/arrow/src/util/bit_util.rs

-        assert_eq!(ceil(10000000000, 10), 1000000000);
-        assert_eq!(ceil(10, 10000000000), 1);
-        assert_eq!(ceil(10000000000, 1000000000), 10);
+        assert_eq!(ceil(10000000, 10), 1000000);


Why the changes here?

I will double check these changes. I made them in response to a test that was failing; however, I found it suspect at the time as well.

The numbers are 64-bit, which won't be supported in wasm32

I think wasm32 supports i64 and f64 https://webassembly.github.io/spec/core/syntax/types.html

paddyhoran · 2020-07-15T02:52:31Z

@rj-atw I would like to see us get wasm support but I think we need to add CI for this. Without CI all those cfg attributes are destined to go stale...

paddyhoran · 2020-07-15T02:55:18Z

Also, I assigned the JIRA to you and changed your permissions so you can self-assign in future

ghost · 2020-07-15T03:01:03Z

@rj-atw I would like to see us get wasm support but I think we need to add CI for this. Without CI all those cfg attributes are destined to go stale...

@paddyhoran I love to add the need CI. Do we have any documentation around this projects CI (especially how to setup env locally)?

sunchao

It will be good to add more details in the PR description, e.g., what have been disabled for wasm32 and what are the consequences etc.

sunchao · 2020-07-15T07:15:19Z

rust/Cargo.toml

@@ -24,3 +24,7 @@ members = [
        "integration-testing",
 	"benchmarks",
 ]
+default-members= [


Why add this? is this related to the change?

Is this just so that wasm builds by default at the top level? If so, I'd rather not do this either. i.e. it's enough to just say that we can build the sub-crates on wasm instead of changing this global default.

Yes. This was add as I did not try and port the Flight and Parquet submodules as I believe their core functionality uses file and sockets which don't have stable support in WASM yet.

I'll try out the individual submodule build, add details to readme and revert the Cargo.toml change.

vertexclique

Made a couple of comments. Nice to see this opened!

vertexclique · 2020-07-15T16:26:41Z

rust/arrow/src/compute/kernels/arithmetic.rs

-#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-fn simd_math_op<T, F>(
+#[cfg(all(any(target_arch = "x86", target_arch = "x86_64", target_arch="wasm32"), feature = "simd"))]
+pub fn simd_math_op<T, F>(


These don't need to be public, they are internally selected based on feature gates.

Thanks. I made this modification to explicitly test that simd op call was not failing in WASM. Forgot to revert while pushing

vertexclique · 2020-07-15T16:27:27Z

rust/arrow/src/lib.rs

 pub mod csv;
 pub mod datatypes;
 pub mod error;
-#[cfg(feature = "flight")]
+#[cfg(all(feature = "flight",not(target_arch="wasm32")))]


Inconsistent spacing between macro features and selections. Please reorganize them consistently.

Also please run cargo fmt and cargo clippy

vertexclique · 2020-07-15T16:29:58Z

rust/datafusion/Cargo.toml

+paste = "0.1"
+
+[target.'cfg(not(target_arch="wasm32"))'.dependencies]
+parquet = { path = "../parquet", version = "1.0.0-SNAPSHOT" }


Don't not gate parquet if not wasm. The parquet should be also wasm compilable. Moreover, actual dependencies shouldn't be gated under the negated set, which makes it harder to maintain.

Will look more into parquet library. I skipped many as it is primarily used to read from the file system...which I believe does not have stable support yet in WASM.

If I change to use a positive gate should I restrict to just x86 and x86_64; or are there other supported targets?

vertexclique · 2020-07-15T16:31:11Z

rust/datafusion/src/bin/repl.rs

+#[cfg(target_arch="wasm32")]
+pub fn main() {
+}


What is the point of not compiling the ported code? I don't see the benefit of porting to wasm by doing this.

WASM is a library only. I believe this code is only useful when creating an executable.

I know this is a bad hack, but I am actually not really sure why this file was still pickup by rustc as I should be excluding in the package dependencies.

I'll take another look at why this is still being compile. However, I am open to suggestion/discussion on alternatives.

paddyhoran · 2020-07-15T18:40:29Z

@paddyhoran I love to add the need CI. Do we have any documentation around this projects CI (especially how to setup env locally)?

If you search for "rust" under the top level "ci" folder you will find most of the CI scripts. We can then ask @kszucs for a quick review of that part of the PR.

kszucs · 2020-07-15T19:02:40Z

@rj-atw you can start here https://github.com/apache/arrow/blob/master/docs/source/developers/docker.rst

I assume you'll need to adjust the rust_build.sh script to accept either a target arch environment variable (preferable) or a CLI argument than add a new docker-compose entry (like debian-rust-wasm32) similar to debian-rust where you pass the wasm variable.

Once that works with archery docker run debian-rust-wasm32 you can add a matrix entry to execute debian-rust-wasm32 in the rust github actions configuration.

emkornfield · 2020-08-27T03:55:54Z

@rj-atw where you able to figure out CI? It looks like this also needs to be rebased.

nevi-me · 2020-11-21T08:28:47Z

Hi @rj-atw, this PR has fallen behind quite a lot. Are you still interested in or able to work on this feature? Otherwise we could close the PR in the interim.

jorgecarleitao · 2020-12-08T06:10:18Z

@rj-atw I am adding wasm32 compilation to the CI, so that this can progress. Is this something that you are still interested on, or should we close this PR?

ghost · 2020-12-08T17:06:51Z

I am still intersted. Started new job, so diverted my attention. But hoping to close this out during holiday break.

…

On Mon, Dec 7, 2020, 10:10 PM Jorge Leitao ***@***.***> wrote: @rj-atw <https://github.com/rj-atw> I am adding wasm32 compilation to the CI, so that this can progress. Is this something that you are still interested on, or should we close this PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7767 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AOPXAG6UVNNQB37QW64C4KDSTW7NPANCNFSM4O2AJQEA> .

alamb · 2020-12-13T13:03:51Z

@rj-atw -- FYI @jorgecarleitao has the appropriate CI support here: #8864 which you can probably pull into this PR

@paddyhoran

This is a requirement fielded to merge #7767, and so I though about adding it to our CI. I am not sure whether this is exactly what we need to test #7767, and so I would like to request help from @paddyhoran and @rj-atw here. NOTE: this is built on top of #8821 . Only the last commit is relevant. Basically, this runs ``` rustup add target wasm32-unknown-unknown cd rust/arrow cargo build --target wasm32-unknown-unknown ``` Note that this does not actually run the tests yet. I think that that is #7767 's goal. Closes #8864 from jorgecarleitao/wasm32 Authored-by: Jorge C. Leitao <[email protected]> Signed-off-by: Andrew Lamb <[email protected]>

alamb · 2021-01-13T13:10:03Z

@rj-atw -- Given the imminent Arrow 3.0 release, I am trying to clean up older Rust PRs and see if the authors have plans to move them forward.

Do you plan on working on this PR in the near future? If not, should we close this PR until there is time to make progress? Thanks again for your contributions so far.

nevi-me · 2021-01-13T14:48:16Z

I'm closing this for now @alamb @rj-atw, we can pick it up later.

@paddyhoran

This is a requirement fielded to merge apache#7767, and so I though about adding it to our CI. I am not sure whether this is exactly what we need to test apache#7767, and so I would like to request help from @paddyhoran and @rj-atw here. NOTE: this is built on top of apache#8821 . Only the last commit is relevant. Basically, this runs ``` rustup add target wasm32-unknown-unknown cd rust/arrow cargo build --target wasm32-unknown-unknown ``` Note that this does not actually run the tests yet. I think that that is apache#7767 's goal. Closes apache#8864 from jorgecarleitao/wasm32 Authored-by: Jorge C. Leitao <[email protected]> Signed-off-by: Andrew Lamb <[email protected]>

Adding Wasm32 compilation support

f6a1caa

paddyhoran reviewed Jul 15, 2020

View reviewed changes

paddyhoran added Component: Rust - DataFusion Component: Rust labels Jul 15, 2020

sunchao reviewed Jul 15, 2020

View reviewed changes

vertexclique suggested changes Jul 15, 2020

View reviewed changes

nevi-me added the needs-rebase A PR that needs to be rebased by the author label Nov 7, 2020

jorgecarleitao mentioned this pull request Dec 7, 2020

ARROW-10838: [Rust] [CI] Add arrow build targeting wasm32 #8864

Closed

nevi-me closed this Jan 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARROW-9453: [Rust] Wasm32 compilation support #7767

ARROW-9453: [Rust] Wasm32 compilation support #7767

ghost commented Jul 15, 2020

github-actions bot commented Jul 15, 2020

paddyhoran Jul 15, 2020

ghost Jul 15, 2020

nevi-me Oct 17, 2020

alippai Oct 17, 2020

paddyhoran commented Jul 15, 2020

paddyhoran commented Jul 15, 2020

ghost commented Jul 15, 2020

sunchao left a comment

sunchao Jul 15, 2020

paddyhoran Jul 15, 2020

ghost Jul 16, 2020

vertexclique left a comment •

edited

Loading

vertexclique Jul 15, 2020

ghost Jul 16, 2020

vertexclique Jul 15, 2020

vertexclique Jul 15, 2020

vertexclique Jul 15, 2020

ghost Jul 16, 2020

vertexclique Jul 15, 2020

ghost Jul 16, 2020

paddyhoran commented Jul 15, 2020 •

edited

Loading

kszucs commented Jul 15, 2020

emkornfield commented Aug 27, 2020

nevi-me commented Nov 21, 2020

jorgecarleitao commented Dec 8, 2020

ghost commented Dec 8, 2020 via email

alamb commented Dec 13, 2020 •

edited

Loading

alamb commented Jan 13, 2021

nevi-me commented Jan 13, 2021

ARROW-9453: [Rust] Wasm32 compilation support #7767

ARROW-9453: [Rust] Wasm32 compilation support #7767

Conversation

ghost commented Jul 15, 2020

github-actions bot commented Jul 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paddyhoran commented Jul 15, 2020

paddyhoran commented Jul 15, 2020

ghost commented Jul 15, 2020

sunchao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vertexclique left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paddyhoran commented Jul 15, 2020 • edited Loading

kszucs commented Jul 15, 2020

emkornfield commented Aug 27, 2020

nevi-me commented Nov 21, 2020

jorgecarleitao commented Dec 8, 2020

ghost commented Dec 8, 2020 via email

alamb commented Dec 13, 2020 • edited Loading

alamb commented Jan 13, 2021

nevi-me commented Jan 13, 2021

vertexclique left a comment •

edited

Loading

paddyhoran commented Jul 15, 2020 •

edited

Loading

alamb commented Dec 13, 2020 •

edited

Loading