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

Simplify the C API #183

Open
5 tasks
edubart opened this issue Feb 6, 2024 · 7 comments
Open
5 tasks

Simplify the C API #183

edubart opened this issue Feb 6, 2024 · 7 comments
Assignees
Labels
refactor Restructuring code, while not changing its original functionality

Comments

@edubart
Copy link
Contributor

edubart commented Feb 6, 2024

Context

Our C API is currently being used to create machine bindings in Lua, Rust, Go and TypeScript languages. Since this, we noticed there are some pain points when creating C bindings in those languages, we could make a refactor to improve it, make it smaller and simpler.

Subtasks

  • Use a JSON string instead of C data structures for loading machines, then deprecate the C config structures. This will make it much easier to bind to other languages, and more proof to mistakes when managing C configs.
  • Combine all write/read CSRs methods into a single method using CSR enumerations, this will make the C API much smaller, simplifying the binding work in case someone is not using a binding generator. We can be even more drastic and combine all read/write methods that fits an uint64 into a single write/read method shared for all registers and CSRs, and use a new enumeration.
  • Stop passing double pointers for error messages in all methods, instead all methods should return an error code, and set an error message that can be optionally inspected with cm_get_last_error_message(machine).
  • Try to minimize the need of some cm_delete_* methods, for example cm_delete_semantic_version could be removed by making it always static allocated.
  • Some dynamic arrays that have bounded limits could also be replaced with fixed arrays, for example doing this with cm_merkle_tree_proof we could remove cm_delete_merkle_tree_proof . Similar we could maybe remove cm_delete_memory_range_config and cm_delete_memory_range_descr_array .
@edubart edubart added the refactor Restructuring code, while not changing its original functionality label Feb 6, 2024
@tuler
Copy link
Member

tuler commented Mar 22, 2024

Those changes would help as well with the Node.js binding I'm working on. I have ~400 lines (probably badly written) only to handle config.

@tuler
Copy link
Member

tuler commented Mar 24, 2024

Side node: cm_reset_uarch should be called cm_reset_uarch_halt_flag, for consistency with other reset methods.

@diegonehab
Copy link
Contributor

That’s not what it does. It really resets the entirety of uarch. Including RAM and all other registers.

@tuler
Copy link
Member

tuler commented Mar 24, 2024

That’s not what it does. It really resets the entirety of uarch. Including RAM and all other registers.

So documentation should be fixed.

/// \brief Resets the value of the microarchitecture halt flag.

@diegonehab
Copy link
Contributor

diegonehab commented Mar 25, 2024

@mpernambuco can you take a look?

@mpernambuco
Copy link
Contributor

@mpernambuco can you take a look?

#211

@GCdePaula
Copy link

GCdePaula commented Mar 28, 2024

I wanted to relate our experience on creating Rust binding.

  • The bindings for Rust in general haven’t been particularly hard to create. Like, the low-level bindings are automatic. Our wrapper to make it idiomatic does require some manual work, but it wasn't particularly hard.
  • It was easy building the machine as part of Rust build pipeline, and linking statically.
  • The read/write register methods were a bit laborious though.
  • Rust deals ok with the double pointer for error messages. In any case, the proposed simplification would help.
  • The runtime config was not a problem to use. Is there a default runtime config? I think I copied "rollups defaults" from cartesi-machine.lua. I also copied some of the asserts.
  • The C machine config structures is a bit unwieldy.

I was initially confused me about the machine config. I think it's because there are instances of it created and managed by us (like in the create function), and there are instances of it created and managed by the emulator. It was easy to deal with the one created and managed by the emulator. I had no problem accessing the fields. No problem freeing memory either, we just need to wrap it in another Rust object and implement Drop correctly.

On using json for machine config. For creating a machine, I think that may help a lot. For querying the machine about the initial config, I think it would not be super ergonomic, but it would be easy, and it may be the best solution

I have a feeling most users will be after the same few fields in the machine config (like the address and length of the tx/rx buffer). Maybe dedicated methods to read commonly needed fields could help?

Or maybe specific functions for common operations, like clearing rx/tx buffer. Or writing an input (setting all the correct registers and buffers). Or reading an input (parsing all the correct registers). I was copying these operations from cartesi-machine.lua. Having them on the API would be more ergonomic, but I don't know if it makes sense. It could give you more freedom to change things though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Restructuring code, while not changing its original functionality
Projects
Status: Todo
Development

No branches or pull requests

5 participants