🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@lastmjs
Copy link

@lastmjs lastmjs commented Apr 15, 2023

The wasm-bindgen stuff is being addressed here: #4875

Also in that PR we are talking about time, but the PR doesn't address a solution to that.

#![allow(clippy::all)]
#![allow(unused)]
include!(concat!(env!("OUT_DIR"), "/python.rs"));
include!("../python.rs");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you give more context behind this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In our environment which is a decentralized Wasm cloud environment, there are function complexity checks that are performed before a Wasm binary is allowed to be deployed. The complexity limit is currently 15_000, but the generated lolrpop python.rs file has a function called __reduce that is ~9000 lines long and has a complexity of 16_722. So for now we need to modify the generated python.rs file and split up the __reduce function. We have asked the protocol engineers of the Wasm environment to increase the complexity limit, but if you know of how to get lolrpop to generate less complex functions that would be great too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but if you know of how to get lolrpop to generate less complex functions that would be great too.

Doesn't seem likely lalrpop/lalrpop#304. Hopefully at some point we should get a PEG parser which shouldn't result in such a behemoth of a function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk if it is possible now, but if you can take input as *.pyc instead of *.py, skipping parsing from the embedded machine will be possible (maybe with little bit fix of rustphython).

/// ```
pub struct Interpreter {
vm: VirtualMachine,
pub vm: VirtualMachine,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto here. What do you use from vm that requires it be public? Can we add an additional method/function to cover the usecase?

Copy link
Contributor

@fanninpm fanninpm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, it's better to make a function disappear than to crash the interpreter.

@lastmjs Does your use case involve running in an interactive shell?

Comment on lines 92 to 118
#[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))]
fn _time(_vm: &VirtualMachine) -> PyResult<f64> {
use wasm_bindgen::prelude::*;
#[wasm_bindgen]
extern "C" {
type Date;
#[wasm_bindgen(static_method_of = Date)]
fn now() -> f64;
#[cfg(feature = "wasmbind")]
{
use wasm_bindgen::prelude::*;
#[wasm_bindgen]
extern "C" {
type Date;
#[wasm_bindgen(static_method_of = Date)]
fn now() -> f64;
}
// Date.now returns unix time in milliseconds, we want it in seconds
return Ok(Date::now() / 1000.0);
}
// Date.now returns unix time in milliseconds, we want it in seconds
Ok(Date::now() / 1000.0)
panic!{"No date available"}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be

#[cfg(all(target_arch = "wasm32", not(target_os = "wasi"), feature = "wasmbind"))]
fn _time(_vm: &VirtualMachine) -> PyResult<f64> {
    // ..
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #4875

Comment on lines 27 to 39
#[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))]
{
use wasm_bindgen::prelude::*;
#[wasm_bindgen]
extern "C" {
#[wasm_bindgen(js_namespace = console)]
fn error(s: &str);
#[cfg(feature = "wasmbind")]
{
use wasm_bindgen::prelude::*;
#[wasm_bindgen]
extern "C" {
#[wasm_bindgen(js_namespace = console)]
fn error(s: &str);
}
let mut s = String::new();
self.write_exception(&mut s, &exc).unwrap();
error(&s);
}
let mut s = String::new();
self.write_exception(&mut s, &exc).unwrap();
error(&s);
panic!("{}; exception backtrace above", msg)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be

#[cfg(all(target_arch = "wasm32", not(target_os = "wasi"), feature = "wasmbind"))]
{
    // ...
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #4875

@lastmjs
Copy link
Author

lastmjs commented Apr 19, 2023

Generally, it's better to make a function disappear than to crash the interpreter.

@lastmjs Does your use case involve running in an interactive shell?

No we are not running in an interactive shell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants