Skip to content

feat: rust performance and api improvements#2080

Open
aclauer wants to merge 19 commits into
mainfrom
andrew/feat/native-rust-perf-improvements
Open

feat: rust performance and api improvements#2080
aclauer wants to merge 19 commits into
mainfrom
andrew/feat/native-rust-perf-improvements

Conversation

@aclauer
Copy link
Copy Markdown
Collaborator

@aclauer aclauer commented May 13, 2026

Problem

Rust module needs some performance improvements and the api could be better.

Closes DIM-XXX

Solution

New api (refer to native/rust/README.md for more docs):

  • Added proc macros in dimos-module-macros crate for message handling and boiler plate (no more select! or match, just write the handlers)
  • inputs/outputs are declared as attributes of the module struct
  • setup and teardown attributes allow for initialization and cleanup

Performance changes:

  • Publish and receive are moved to separate tokio tasks so they can't block each other
  • Routing is done with hashmap instead of linear search through topics. Now routing takes constant time.

Tests:

  • Added tests for the publish / receive concurrency blocking

Random:

  • Renamed crates to dimos-module and dimos-module-macros
  • Added cargo clippy and cargo fmt to precommit. In future should add CI testing.

How to Test

cargo test
python3 rust_ping_pong.py

Contributor License Agreement

  • I have read and approved the CLA.

@aclauer aclauer changed the title Andrew/feat/native rust perf improvements feat: rust performance and api improvements May 13, 2026
@aclauer aclauer marked this pull request as ready for review May 13, 2026 19:43
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 13, 2026

Greptile Summary

This PR refactors the Rust native module SDK into a proper Cargo workspace (dimos-module + dimos-module-macros), adds a #[derive(Module)] proc-macro that eliminates select!/match boilerplate, and improves performance by splitting publish and recv into separate independent Tokio tasks with O(1) HashMap routing.

  • API overhaul: modules are now declared as structs with #[input]/#[output]/#[config] field attributes and handler methods named handle_<field>; run() replaces the manual from_stdin + spawn pattern.
  • Performance: recv and publish tasks run independently so a slow transport publish cannot stall incoming message dispatch; routing is now O(1) via HashMap<topic, Vec<Box<dyn Route>>>.
  • Lifecycle: task JoinHandles are awaited inside run()'s select!; teardown() always executes before failure propagation; new concurrency tests verify the independent-task properties directly.

Confidence Score: 5/5

Safe to merge; the lifecycle and concurrency concerns from previous rounds are addressed and the new code is well-tested.

The two independent-task split eliminates the old publish/recv mutual blocking, teardown now always runs before failure propagation, and the task handles are properly awaited in the select!. Tests cover the concurrency properties directly. The remaining findings are style-level: a missing where bound on generated state-field initializers and a narrow gap in the pre-commit hook trigger condition. Neither affects runtime correctness.

No files require special attention; the macro and runtime code are consistent with each other and with the documented examples.

Important Files Changed

Filename Overview
native/rust/dimos-module/src/module.rs Core runtime: Module trait, Builder, spawn_pubsub_tasks, run(). Handles are now properly awaited in select! and teardown always runs before failure propagation. A state field with a non-Default type will produce an opaque compile error pointing into the macro-generated build() body.
native/rust/dimos-module-macros/src/lib.rs New proc-macro crate: parses #[input], #[output], #[config], #[module] attributes and generates the Module impl. Well-structured with good error messages for invalid attribute combinations. State fields get Default::default() with no generated where bound, so missing Default gives an unhelpful error.
.pre-commit-config.yaml Adds cargo-fmt --check and cargo-clippy -D warnings hooks. Correctly de-duplicates workspace roots via sort -u. Hooks are gated on types: [rust] so a Cargo.toml-only commit won't trigger them.
native/rust/dimos-module/src/transport.rs Transport trait gains Sync bound and recv() changes to &self to support Arc-sharing between the two independent tasks. Clean breaking change that matches the new spawn_pubsub_tasks design.
examples/native-modules/rust/src/native_ping.rs Rewritten to use the new derive(Module) API. Publisher spawned in setup() as a background task; handle_confirm prints received echoes. Clean.
examples/native-modules/rust/src/native_pong.rs Rewritten to use derive(Module) with a typed PongConfig and a handle_data method. Substantially simpler than before.
native/rust/README.md New documentation covering struct attributes, lifecycle steps, and an annotated macro-expansion example. Accurate and well-organized.

Sequence Diagram

sequenceDiagram
    participant Main
    participant RunFn as "run()"
    participant Builder
    participant RecvTask
    participant PubTask
    participant Transport

    Main->>RunFn: "run::<M,T>(transport)"
    RunFn->>RunFn: "read JSON from stdin (topics + config)"
    RunFn->>Builder: "Builder::new(topics, publish_tx)"
    RunFn->>Builder: "M::build(&mut builder, config)"
    Builder-->>RunFn: "module (Input/Output fields populated)"
    RunFn->>RecvTask: "tokio::spawn recv loop"
    RunFn->>PubTask: "tokio::spawn publish loop"
    RunFn->>RunFn: "module.setup().await"

    loop "module.handle() select! over inputs"
        RecvTask->>Transport: "transport.recv().await"
        Transport-->>RecvTask: "(channel, data)"
        RecvTask->>RecvTask: "routes[channel].try_dispatch(data)"
        RecvTask->>RunFn: "Input channel receives decoded message"
        RunFn->>RunFn: "handle_field(msg).await"
        RunFn->>PubTask: "Output::publish sends to publish_tx"
        PubTask->>Transport: "transport.publish(topic, bytes)"
    end

    alt "ctrl-c OR handle() exits OR task panic"
        RunFn->>RunFn: "module.teardown().await"
        RunFn->>RunFn: "propagate_task_failure if panicked"
        RunFn-->>Main: "Ok(()) or panic"
    end
Loading

Reviews (8): Last reviewed commit: "Teardown even with error" | Re-trigger Greptile

Comment thread native/rust/dimos-module/src/module.rs
Comment thread native/rust/dimos-module/src/module.rs
Comment thread native/rust/dimos-module/src/module.rs
Comment thread native/rust/README.md Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Comment thread native/rust/README.md Outdated
Comment thread .pre-commit-config.yaml
Comment on lines +112 to +117
- id: cargo-fmt
name: cargo fmt
entry: bash -c "cargo fmt --manifest-path native/rust/Cargo.toml --all --check && cargo fmt --manifest-path examples/native-modules/rust/Cargo.toml --check"
language: system
types: [rust]
pass_filenames: false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Dreamsorcerer example of why I care about single path for CI and local dev envs for linting/code modification, think we should do what we discussed with modifying vs validating pre-commit hooks?

Co-authored-by: Paul Nechifor <paul@nechifor.net>
Comment thread native/rust/dimos-module/src/module.rs Outdated
Comment thread native/rust/dimos-module/src/module.rs Outdated
Comment thread .pre-commit-config.yaml Outdated
Comment thread native/rust/dimos-module/src/module.rs
Comment thread native/rust/dimos-module/src/module.rs
Comment thread native/rust/dimos-module/src/module.rs Outdated
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.

3 participants