This is a draft to show our remaining reasons for having a fork of RustPython#4878
This is a draft to show our remaining reasons for having a fork of RustPython#4878lastmjs wants to merge 7 commits intoRustPython:mainfrom
Conversation
| #![allow(clippy::all)] | ||
| #![allow(unused)] | ||
| include!(concat!(env!("OUT_DIR"), "/python.rs")); | ||
| include!("../python.rs"); |
There was a problem hiding this comment.
could you give more context behind this?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
ditto here. What do you use from vm that requires it be public? Can we add an additional method/function to cover the usecase?
vm/src/stdlib/time.rs
Outdated
| #[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"} | ||
| } |
There was a problem hiding this comment.
This should be
#[cfg(all(target_arch = "wasm32", not(target_os = "wasi"), feature = "wasmbind"))]
fn _time(_vm: &VirtualMachine) -> PyResult<f64> {
// ..
}
vm/src/vm/vm_object.rs
Outdated
| #[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) | ||
| } |
There was a problem hiding this comment.
This should be
#[cfg(all(target_arch = "wasm32", not(target_os = "wasi"), feature = "wasmbind"))]
{
// ...
}
No we are not running in an interactive shell |
b64a9f9 to
b7b0a49
Compare
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.