SkylerLipthay / mini-v8

A minimal embedded V8 JavaScript engine wrapper for Rust
MIT License
104 stars 11 forks source link

Rewrite using rusty_v8 #3

Closed SkylerLipthay closed 11 months ago

SkylerLipthay commented 4 years ago

The hard workers over at deno/rusty_v8 solved the issue of bundling/building V8, so we should definitely be taking advantage of that. This also means I get to delete all of the ugly and possibly incorrect C++ code in MiniV8!

jkelleyrtp commented 4 years ago

Curious what would be involved in that - do you have a list of items that external contributors could help with?

Awesome crate nonetheless, really useful!

SkylerLipthay commented 4 years ago

I appreciate your interest! Honestly I don't know what I'm doing when it comes to directing open source contributions. This refactor is something I was planning on tackling alone in the next couple of weeks as this crate has become important for me again. I'll keep this issue updated as I investigate further and come up with a game plan.

Vrganj commented 4 years ago

I would love that, is it being worked on?

SkylerLipthay commented 4 years ago

I haven't gotten around to writing a proper post-mortem. Long story short I attempted this a rewrite a couple weeks ago, see the rusty_v8-rewrite branch. I banged my head against the wall for a few days trying to get this crate's API to work with rusty_v8.

The primary problem is that rusty_v8's scope handle model is less flexible than V8's own. In rusty_v8, isolates are borrowed mutably when they issue a handle scope. This is a major issue because we need e.g. mini_v8::Objects to be able to grab handle scopes regardless of the state of the isolate being borrowed already by the parent MiniV8. This becomes particularly problematic during ToValue/FromValue conversion.

The secondary problem is that rusty_v8 is still immature and I don't have the personal resources to contribute much. I did submit a PR to implement a few V8 APIs, but there are some big APIs that this crate needs like V8's Externals/Privates that I'm not comfortable mapping according to rusty_v8's potential needs... I noticed that rusty_v8's lead dev was pushing some stuff related to Externals recently but I haven't kept up.

I'm thinking this is worth reattempting further along in rusty_v8's life.

In the meantime I rewrote this crate's FFI to hopefully eliminate correctness issues and I updated it to the latest V8 version.

Lioncat2002 commented 1 year ago

Any updates on this? Compiling V8 is a pain and mini-v8 provides a very good interface with v8.

SkylerLipthay commented 1 year ago

Thanks for your interest and also the kind words about mini-v8's interface. I haven't thought about this crate in a while, and I don't have any immediate plans for working on it.

Glancing at rusty_v8, it seems that now it is more than capable of serving as a V8 binary provider and as a safe FFI for use by mini-v8. I could definitely see mini-v8 being rewritten as a thin layer over rusty_v8. Perhaps I'll give it a go when I have some free time, but until then I'll definitely support any efforts by you or anyone else to revitalize this crate and take it in that direction.

e: I just reviewed my post-mortem above. rusty_v8 has support for External/Private now, but I believe there still exists the mutable isolate borrow problem. Regardless, I believe the mini-v8 interface is still viable in tandem with rusty_v8, even if some Rcs need to be introduced or whatever.

Lioncat2002 commented 1 year ago

I am not much of a rust guy But I can definitely give it a try once I get a few things off my hand! :)

SkylerLipthay commented 1 year ago

I just tackled this. The MiniV8 API remains almost untouched. There are some rough edges, but unsafe has been reduced from ~40 to ~5 instances. The unsafe usage exists only to translate rusty_v8's stack-based model (see my comment from July 2020) to MiniV8's heap-based model. It needs auditing. Until then, this crate remains unready for production.

But still, depending on the MiniV8 crate is now very simple, and the V8 dependency is as up-to-date as rusty_v8's dependency on V8 is. 😄