fix(plugin-runner): More details on pointer conversion failures

[stahlbauer]

Description: This change set provides (1) additional debug infos in cases when conversions of pointers between the Rust world and the WASM world fail, and (2) fixes an existing misleading error message.

Related issue (if exists): This should help to make progress addressing #6249.

[CLAassistant]

All committers have signed the CLA.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[kdy1]

You can build by yourself and replace node binding to check the cause. This PR is not required

[stahlbauer]

@kdy1 : Thanks for this hint. Sounds like a good approach. I guess that the steps performed in

swc/.github/workflows/publish-node.yml

Line 54 in 410ec6f

build: >-

are needed for building this for Linux/Intel64? This is related to the discussion started in #6284.

[kdy1]

You need --features plugin

[stahlbauer]

Thanks for the hint on the plugin feature. That is, actually the musl target has to be build to work with Napi?

swc/.github/workflows/publish-node.yml

Line 62 in 410ec6f

target: x86_64-unknown-linux-musl

[kdy1]

I'm sorry, but I didn't get what you are saying. Can you explain in other words? (sorry for my bad English, I'm not an English native)

[stahlbauer]

We are trying to build the SWC binaries such that they can be used within the NodeJs package, that is, to copy them in the end into ./node_modules/@swc/core of our NodeJs project to use our the build of SWC with the modifications we made for debugging purposes.

I think that having a list of steps to perform (starting with a fork of the SWC repo with some modifications) for such debugging endeavors would be helpful for the community as a whole. At least, I would appreciate it ;)

I am on a recent Linux machine with Ubuntu and 64bit.

[stahlbauer]

One problem I am facing at the moment is that the SWC crates are used from the registry and not from the monorepo itself. (I am neither English nor Rust native ;))

[stahlbauer]

The snippet

716       - name: Patch                                                                                                     
717         run: |                                                                                                          
718           echo '[patch.crates-io]' >> bindings/Cargo.toml                                                               
719           ./scripts/cargo/patch-section.sh >> bindings/Cargo.toml                                                       
720           cd bindings && cargo update  

from the CI.yml seems to be relevant to ensure that local Crates are used in the build process and not queried from the registry.

[stahlbauer]

By patching the Cargo.toml as indicated above, I was able to use my own SWC binary. The plugin runner now fails with:

thread '<unnamed>' panicked at 'Should be able to convert the value -2147188304 to u32', /swc-fork/crates/swc_plugin_runner/src/memory_interop.rs:37:29

This is the adjusted output proposed by this pull request.

[kdy1]

cc @kwonoj Any thoughts? I think it's -1. So it can be 32bit (wasm) vs 64 bit (host) I guess, but not sure if it's the case.

[kwonoj]

Yes, seems like it's some kind overflow peeking by number only, but to confirm we need to see if host created number larger than u32 really. I'm not sure either if memory interop supposed to create 64 as allocation happens in wasm runtime would create wasm's memory boundary.

[stahlbauer]

Would it make sense to use u32 for pointer values instead of i32? I am missing some Rust and WASM background here (wasm32 is supposed to support up to 4GB of memory).

The code uses i32 for pointers where I would expect u32 to be more appropriate, for example:

#[tracing::instrument(level = "info", skip_all)]
pub fn allocate_return_values_into_guest(
    memory: &Memory,
    alloc_guest_memory: &NativeFunc<u32, i32>,
    allocated_ret_ptr: i32,
    serialized_bytes: &PluginSerializedBytes,
) {

Also here:

#[cfg(feature = "__rkyv")]
#[cfg_attr(not(target_arch = "wasm32"), allow(unused))]
#[tracing::instrument(level = "info", skip_all)]
pub fn read_returned_result_from_host_fallible<F, R>(f: F) -> Option<R>
where
    F: FnOnce(i32) -> i32,
    R: rkyv::Archive,
    R::Archived: rkyv::Deserialize<R, rkyv::de::deserializers::SharedDeserializeMap>,
{

[stahlbauer]

I have changed the pointer types to u32 locally and things got better. Now I touched the 4GB barrier and got OOM errors. The memory graph with several invocations of SWC looks as follows:

I have produced this plot using mprof, a set of Python scripts that can also record memory consumed by child processes; it performs memory sampling.

[kdy1]

Are you facing it with next dev server?

[kdy1]

I mean, a long-running process

[stahlbauer]

I am facing the problem when instrumenting a larger JS bundle with https://github.com/kwonoj/swc-plugin-coverage-instrument . I am invoking that plugin programatically. I am doing some additional transformations on the resulting AST.

[stahlbauer]

The process is short lived.

[kdy1]

I'm aware of the memory issue, but I didn't think someone would process a such big input

[kdy1]

How big is it? What's the sum of all inputs to swc within a single process?

[stahlbauer]

The file that is passed to SWC has 500KB (only) and is a small react application that was bundled using Vite/ESbuild. I would have bundles of 100MBs and more to work on.

[stahlbauer]

(The size of the bundles to work on is actually the reason to use SWC instead of Babel).

[kdy1]

Hmm... Then it's not the issue I was aware of

[stahlbauer]

What do you think about changing the pointers from i32 to u32 in a first step? Does this make sense from your perspective?

[stahlbauer]

At least, the real cause of the problem is no longer masked then:

    1: RuntimeError: unreachable
           at abort (<module>[35215]:0x935d9c)
           at std::sys::wasi::abort_internal::h58865237df468a7d (<module>[35129]:0x92f141)
           at std::process::abort::h74127d44f02b51e4 (<module>[35162]:0x9315d9)
           at rust_oom (<module>[35172]:0x931bb5)
           at __rg_oom (<module>[35260]:0x9379e3)
           at __rust_alloc_error_handler (<module>[12390]:0x431f7b)
           at alloc::alloc::handle_alloc_error::rt_error::he27203bd7d9482b1 (<module>[35249]:0x9374ba)

[stahlbauer]

Wasm64 would be the the best thing to have :D

[kdy1]

I think it's fine, but I want to ask @kwonoj about opinions

[kwonoj]

I think it is no harm to try, at least it shouldn't be a breaking changes. But as discussed above, if the input size is real culprit we don't have lot of options to solve it. wasm64 is probably not available in a short timeframe.

[stahlbauer]

@kdy1 @kwonoj I have pushed my changes. In case you would like to integrate my changes, I would welcome a review and would also be willing to do the necessary rework.

[kdy1]

swc-bump:

• swc_common

[kdy1]

@stahlbauer Please allow editing PR

[stahlbauer]

@kdy1 : You should have the permissions now. Thanks.

[stahlbauer]

As a follow-up, I have created the following ticket: #6404

[kdy1]

CI failed

[stahlbauer]

@kdy1: I will have a look.