Skip to content

Commit e04f936

Browse files
awesomebytesclaude
andcommitted
Wire the per-type subscription cache into RclcppyyNode.create_subscription
The startup-JIT lane on cppyy_kit (subscription cache in rclcpp_kit) removes the ~2.8 s cppyy JIT of create_subscription<MsgT>; rclcppyy has to opt in to benefit. RclcppyyNode.create_subscription previously did two uncacheable JITs per call: a per-UUID cppyy.cppdef of a <Python.h> lambda trampoline, and the raw create_subscription[MsgT] template. Both are replaced by: * cpp_callback = std::function<void(std::shared_ptr<const MsgT>)>(callback) built directly via cppyy -- it delivers the C++ message proxy straight to the Python callback and manages the GIL, so the old raw-pointer rebinding trampoline is gone; and * rclcpp_kit.subscription_cache.make_subscription(node, cpp_type, header, topic, qos, cpp_callback), which loads a prebuilt per-type .so (or schedules a background build and returns None -> plain-template fallback this run). The header is derived from the resolved C++ type string (_subscription_header, rosidl CamelCase->snake) rather than rclcpp_kit.message_header: under acceleration the message class is already a cppyy C++ type, for which message_header returns None. subscription_cache is absent on the published suite 0.1.0, so the whole path is guarded and falls back to the plain create_subscription[MsgT] call there (pub/sub roundtrip tests still pass in the default env). Correctness first; the cache is a pure speedup. Measured on the real `ros2 topic hz` (demo-topic-hz-startup, time to first hz line): stock 1.7 s; RCLCPPYY_ENABLE_HOOK=1 cold 8.3 s, warm 4.1 s. The subscription cache cut the warm start ~7.0 s -> 4.1 s (the eliminated create_subscription<Image> JIT); the residual gap over stock is rclcpp bring-up (cppyy symbol discovery + adapters), not yet cached. Delivery verified: hooked ros2 topic hz reports exact rates. pixi run test: 32 passed. pixi run lint: clean. README startup section updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0bbeeea commit e04f936

3 files changed

Lines changed: 94 additions & 90 deletions

File tree

README.md

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -281,21 +281,23 @@ stock rclpy crosses one full core near 200–250 Hz (92–107 %), while the acce
281281
backend stays around a third of a core, so on a busier machine or at a higher rate
282282
the stock reader saturates first.
283283

284-
**Startup: the acceleration is a one-time cost.** Time-to-first-hz-line
284+
**Startup: a one-time cost.** Time to first hz line
285285
(`pixi run -e heavydemo demo-topic-hz-startup`):
286286

287287
| `ros2 topic hz` first hz line | time |
288288
|---|---|
289289
| stock (no acceleration) | ~1.7 s |
290-
| `RCLCPPYY_ENABLE_HOOK=1`, cold PCH cache | ~8.0 s |
291-
| `RCLCPPYY_ENABLE_HOOK=1`, warm PCH cache | ~7.0 s |
292-
293-
The suite's zero-config Cling PCH shaves ~1.1 s (the `rclcpp` header parse) off the
294-
first line, cold→warm; the remaining ~5 s over stock is one-time rclcpp bring-up
295-
plus first-use JIT of the message type and subscription glue, which the header PCH
296-
does not cover. So acceleration adds a multi-second startup and **pays off for
297-
long-running `hz` sessions**, not short invocations. (Needs the newer auto-PCH; see
298-
the dev-bridge note below.)
290+
| `RCLCPPYY_ENABLE_HOOK=1`, cold cache | ~8.3 s |
291+
| `RCLCPPYY_ENABLE_HOOK=1`, warm cache | ~4.1 s |
292+
293+
Two caches cut the warm start: the suite's Cling PCH removes the `rclcpp` header
294+
parse, and a per-message-type subscription cache removes the ~2.8 s one-time JIT of
295+
`create_subscription<MsgT>` (both built on the first run and loaded on later ones,
296+
cold → warm above). The warm figure is still above stock because the rest of the
297+
rclcpp bring-up — cppyy symbol discovery and the rclpy-compatibility adapters — is
298+
not yet cached. Acceleration therefore remains a one-time startup cost that pays off
299+
over a long-running `hz` session rather than a single short call. (Needs the newer
300+
suite; see the dev-bridge note below.)
299301

300302
### Large messages under BEST_EFFORT QoS
301303

rclcppyy/node.py

Lines changed: 61 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import inspect
2+
import re
23
import uuid
34
from typing import Union, Optional, Callable, Any, List
45
import rclpy
@@ -16,6 +17,25 @@
1617
from rclcppyy import bringup_rclcpp
1718
from rclcppyy.bringup_rclcpp import _resolve_message_type, _is_msg_python, convert_python_msg_to_cpp
1819

20+
21+
def _subscription_header(cpp_type_str):
22+
"""C++ header path for a resolved message type string (e.g.
23+
``sensor_msgs::msg::Image`` -> ``sensor_msgs/msg/image.hpp``), for the
24+
subscription cache's ``#include``. Derived from the type string because under
25+
acceleration the message class is already a C++ (cppyy) type, for which
26+
rclcpp_kit.message_header returns None. Uses rosidl's CamelCase->snake_case
27+
convention (``PointCloud2`` -> ``point_cloud2``, ``TFMessage`` -> ``tf_message``).
28+
Returns None if the string isn't a ``pkg::msg::Name`` triple.
29+
"""
30+
parts = cpp_type_str.split("::")
31+
if len(parts) != 3:
32+
return None
33+
package, _, name = parts
34+
snake = re.sub(r"([a-z0-9])([A-Z])", r"\1_\2",
35+
re.sub(r"(.)([A-Z][a-z]+)", r"\1_\2", name)).lower()
36+
return "%s/msg/%s.hpp" % (package, snake)
37+
38+
1939
class RclcppyyNode(Node):
2040
"""
2141
This node inherits from rclpy Node but it uses rclcpp for publishers/subscribers.
@@ -75,28 +95,7 @@ def __init__(self,
7595
"""
7696
self._cpp_timers = {}
7797

78-
# Subscription callback template for dynamic C++ callback generation
79-
self._subscription_cpp_template = """
80-
#include <Python.h>
81-
#include <functional>
82-
83-
static std::function<void(const PLACEHOLDER_MSG_TYPE::SharedPtr)>
84-
create_subscription_callback_PLACEHOLDER_UUID(PyObject* self) {
85-
return [self](const PLACEHOLDER_MSG_TYPE::SharedPtr msg) {
86-
if (self && PyObject_HasAttrString(self, "_subscription_callback_PLACEHOLDER_UUID")) {
87-
// Acquire the GIL before calling into Python to avoid crashes
88-
PyGILState_STATE gstate = PyGILState_Ensure();
89-
// Pass the raw pointer as a Python int; Python will bind it back to a proxy
90-
PyObject* py_ptr = PyLong_FromVoidPtr(reinterpret_cast<void*>(msg.get()));
91-
PyObject_CallMethod(self, "_subscription_callback_PLACEHOLDER_UUID", "O", py_ptr);
92-
Py_XDECREF(py_ptr);
93-
PyGILState_Release(gstate);
94-
}
95-
};
96-
}
97-
"""
9898
self._cpp_subscriptions = {}
99-
self._subscription_callbacks = {}
10099

101100
# Now we create the rclpy node
102101
super().__init__(node_name,
@@ -310,74 +309,56 @@ def create_subscription(self,
310309

311310
# Convert rclpy message to rclcpp message type
312311
rclcpp_msg_type, rclcpp_msg_class = _resolve_message_type(msg_type)
313-
# We could make this optional for efficiency
314-
# add_python_compatibility(cpp_msg, msg_type)
315-
316-
# Generate a unique UUID for the subscription
317-
uuid_str = str(uuid.uuid4()).replace("-", "_")
318-
319-
# Replace placeholders in the C++ template
320-
cpp_subscription_template = self._subscription_cpp_template.replace(
321-
"PLACEHOLDER_MSG_TYPE", rclcpp_msg_type
322-
).replace("PLACEHOLDER_UUID", uuid_str)
323-
324-
# Compile the C++ callback wrapper
325-
cppyy.cppdef(cpp_subscription_template)
326-
327-
# Store a Python wrapper that translates raw pointer (int) back to C++ proxy and calls user callback
328-
def _cpp_bound_subscription_callback(py_ptr):
329-
try:
330-
# Resolve cpp class for binding
331-
# _, cpp_msg_class = _resolve_message_type(msg_type)
332-
# Rebind raw pointer (Python int) to a cppyy proxy
333-
cpp_msg = cppyy.bind_object(py_ptr, rclcpp_msg_class)
334-
callback(cpp_msg)
335-
except Exception as e:
336-
print(f"Error in subscription callback: {e}")
337-
import traceback
338-
traceback.print_exc()
339-
340-
# Store the wrapper on the node instance
341-
setattr(self, f"_subscription_callback_{uuid_str}", _cpp_bound_subscription_callback)
342-
343-
# Create the C++ callback wrapper
344-
cpp_callback = getattr(cppyy.gbl, f"create_subscription_callback_{uuid_str}")(self)
345-
346-
# Create subscription options for rclcpp
347-
options = self._rclcpp.SubscriptionOptionsWithAllocator["std::allocator<void>"]()
348-
if callback_group:
349-
# TODO: Convert callback group from rclpy to rclcpp if needed
350-
pass
351-
if qos_overriding_options:
352-
# TODO: Handle QoS overriding options
353-
pass
354-
312+
313+
# Build the C++ callback directly as a std::function. cppyy delivers the C++
314+
# message proxy straight to the Python callback and manages the GIL, so no
315+
# per-subscription C++ trampoline (the old per-UUID <Python.h> lambda that
316+
# rebound a raw pointer) is needed.
317+
cpp_callback = cppyy.gbl.std.function[
318+
"void(std::shared_ptr<const %s>)" % rclcpp_msg_type
319+
](callback)
320+
355321
# Convert QoS profile to rclcpp QoS
356322
rclcpp_qos = self._convert_qos_to_rclcpp(qos_profile)
357-
358-
# Create the actual rclcpp subscription
359-
subscription = self._rclcpp.create_subscription[rclcpp_msg_type](
360-
self._rclcpp_node,
361-
final_topic,
362-
rclcpp_qos,
363-
cpp_callback,
364-
options
365-
)
366-
367-
# Store the subscription and callback reference
323+
324+
# Route through the cached per-type trampoline so the ~2.8 s
325+
# create_subscription<MsgT> template instantiation JITs once into a .so
326+
# instead of on every process start. Falls back to the plain template call
327+
# when no .so is built yet (this run), for callback groups / QoS overrides,
328+
# or when the message header can't be derived -- correctness first, the cache
329+
# is a pure speedup.
330+
subscription = None
331+
if callback_group is None and qos_overriding_options is None:
332+
try:
333+
# subscription_cache postdates rclcpp_kit 0.1.0; on an older suite it
334+
# is absent and we simply use the plain path.
335+
from rclcpp_kit import subscription_cache
336+
header = _subscription_header(rclcpp_msg_type)
337+
if header:
338+
subscription = subscription_cache.make_subscription(
339+
self._rclcpp_node, rclcpp_msg_type, header, final_topic,
340+
rclcpp_qos, cpp_callback)
341+
except ImportError:
342+
subscription = None
343+
if subscription is None:
344+
options = self._rclcpp.SubscriptionOptionsWithAllocator["std::allocator<void>"]()
345+
subscription = self._rclcpp.create_subscription[rclcpp_msg_type](
346+
self._rclcpp_node, final_topic, rclcpp_qos, cpp_callback, options)
347+
348+
# Keep the Python callback and its std::function wrapper alive for the
349+
# subscription's lifetime; dropping them would dangle the C++ callback.
368350
self._cpp_subscriptions[subscription] = {
369351
'callback': callback,
370-
'uuid': uuid_str,
371-
'msg_type': rclcpp_msg_type
352+
'cpp_callback': cpp_callback,
353+
'msg_type': rclcpp_msg_type,
372354
}
373-
self._subscription_callbacks[uuid_str] = callback
374-
355+
375356
# Add to subscriptions list for compatibility
376357
self._subscriptions.append(subscription)
377-
358+
378359
# Wake executor like rclpy does
379360
self._wake_executor()
380-
361+
381362
return subscription
382363

383364
def destroy_node(self):

scripts/heavy_hz_demo/run_topic_hz_cli.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,12 +295,18 @@ def startup_story(args):
295295
log("Building the PCH cache with a clean-exiting helper ...")
296296
_warm_pch_cache(story_cache, args.echo)
297297
_wait_pch(story_cache)
298+
# The hz path also JIT-instantiates create_subscription<Image> (~2.8 s); build
299+
# its cached trampoline .so so the warm run loads it too.
300+
log("Building the subscription-trampoline cache ...")
301+
_prebuild_subscription(story_cache, "sensor_msgs::msg::Image",
302+
"sensor_msgs/msg/image.hpp", args.echo)
298303
log("[WARM] PCH cached -> `RCLCPPYY_ENABLE_HOOK=1 ros2 topic hz` ...")
299304
warm = _run_hz(topic, args.window, True, 2.0, max(args.warmup_timeout, 120.0),
300305
story_cache, args.echo)
301306
log("[WARM] time to first hz line: %ss" % warm["ttfl"])
302307
log("[PLAIN] stock `ros2 topic hz` (no acceleration) for reference ...")
303308
plain = _run_hz(topic, args.window, False, 2.0, args.warmup_timeout, None, args.echo)
309+
log("[PLAIN] time to first hz line: %ss" % plain["ttfl"])
304310
print()
305311
print(" `ros2 topic hz` time-to-first-hz-line")
306312
print(" " + "=" * 52)
@@ -313,6 +319,21 @@ def startup_story(args):
313319
shutil.rmtree(story_cache, ignore_errors=True)
314320

315321

322+
def _prebuild_subscription(cache_dir, cpp_type, header, echo):
323+
"""Synchronously build the subscription-trampoline .so into cache_dir so the warm
324+
run loads it instead of JIT-instantiating create_subscription<MsgT>."""
325+
env = dict(os.environ)
326+
env["XDG_CACHE_HOME"] = str(cache_dir)
327+
env.pop("CLING_STANDARD_PCH", None)
328+
env.pop("_CPPYY_KIT_AUTOPCH_ACTIVE", None)
329+
p = Child("subprebuild", [sys.executable, "-u", "-m", "rclcpp_kit._sub_prebuild",
330+
cpp_type, header], env, echo=echo)
331+
deadline = time.monotonic() + 180.0
332+
while time.monotonic() < deadline and p.alive():
333+
time.sleep(0.5)
334+
p.stop()
335+
336+
316337
def _warm_pch_cache(cache_dir, echo):
317338
"""Bring up acceleration in a throwaway process that exits cleanly, so the
318339
auto-PCH build (scheduled at interpreter exit) actually runs and populates

0 commit comments

Comments
 (0)