Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generate rust somehow to uniformly define how a struct gets "deserialized" into python -- potential perf boost #7371

Closed
2 tasks
cosmicexplorer opened this issue Mar 12, 2019 · 4 comments

Comments

@cosmicexplorer
Copy link
Contributor

cosmicexplorer commented Mar 12, 2019

We add zipkin spans to the rust code in the v2 engine in #7342, and one of the things we get from that is a dict of the new rust WorkUnit struct. Two things could potentially be made easier:

  • Rust structs could specify how they get mapped to a dict or list declaratively, which could be more readable than having to inspect a vector of externs::store_* and visualize how the dict is created in your head.
  • We could avoid calling externs::store_* methods one after another, which requires going back and forth over the FFI boundary many times to store a rust WorkUnit, even for statically-known information such as dict keys for each field!

If we were to support having python code know more about the structure of rust objects it receives, we could consider declaring some schema in python code in native.py which python code would then generate a C struct from, then generating a rust object, possibly using the bindgen crate. Then, we could envision something like an externs::store_python_object() function we call from rust: see #7367 for a related idea. We would probably need to have some id for each schema we specify in python in order to have the extern_store_python_object() method we would add in native.py know how to deserialize the specific object it's getting.

Alternatively, if going back and forth over the FFI boundary is about as fast as trying to generate a python dict all in one go (the second checkbox above), then we could avoid trying to do anything in python, and just try to satisfy the first checkbox by using either rust macros or a trait somehow to make converting rust objects into python objects more declarative.

@stuhood
Copy link
Sponsor Member

stuhood commented Mar 12, 2019

. @illicitonion experimented with using flatbuffers a while back, although not in the context of pants.

But I'm not sure when to do something like this vs switching to embedding starlark... embedding starlark would net us "completely identical strings" and "rust-introspectable built-in structs" for example. See https://docs.rs/starlark/0.2.0/starlark/values/index.html

@cosmicexplorer
Copy link
Contributor Author

It seems like still have to swap starlark over to python at some point though to interact with the v2 engine, which is another type of FFI, unless we just want to use bazel.

@cosmicexplorer
Copy link
Contributor Author

cosmicexplorer commented Mar 13, 2019

Well, modulo #7093, maybe (not the description, but the bottom).

@cosmicexplorer cosmicexplorer changed the title generate rust somehow to uniformly define how a struct gets "deserialized" into python generate rust somehow to uniformly define how a struct gets "deserialized" into python -- potential perf boost Mar 15, 2019
@stuhood
Copy link
Sponsor Member

stuhood commented Jun 8, 2020

This was resolved by #9593: we can now directly interact with Python objects from rust (with the GIL acquired), and so many objects that cross the boundary can now be "Python-wrapped" cpython-compatible objects.

Example: Session is a rust object with a python constructor and no fields exposed:

py_class!(class PySession |py| {
data session: Session;
def __new__(_cls,
scheduler_ptr: PyScheduler,
should_record_zipkin_spans: bool,
should_render_ui: bool,
build_id: String,
should_report_workunits: bool
) -> CPyResult<Self> {
Self::create_instance(py, Session::new(
scheduler_ptr.scheduler(py),
should_record_zipkin_spans,
should_render_ui,
build_id,
should_report_workunits,
)
)
}
});

@stuhood stuhood closed this as completed Jun 8, 2020
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

No branches or pull requests

2 participants