Skip to content

Commit 2768118

Browse files
pabloinigoblascofacontidavideclaude
authored
refactor(object_store): lazy fetch returns PayloadView (#103)
* refactor(object_store): lazy fetch returns PayloadView; payload slot uses std::any The lazy resolver registered with ObjectStore::pushLazy now returns a PayloadView (Span + BufferAnchor) instead of a std::vector<uint8_t>. This lets producers hand off bytes they already hold behind a shared_ptr (e.g. a streaming buffer being reused across stores) without copying — the returned anchor extends the buffer's lifetime through the read. For producers whose only payload is a plain std::vector<uint8_t>, the new sdk::makePayloadView() helper in pj_base wraps the vector into a shared_ptr (which becomes both owner and type-erased anchor) and returns a PayloadView pointing at its contents. This is also the contract resolveEntry() relies on: it recovers the shared_ptr<const vector> from the anchor via static_pointer_cast. ObjectEntry::payload now stores a std::any instead of a std::variant<shared_ptr, std::function>. The dispatch in ObjectStore::resolveEntry uses std::any_cast for each branch. Rationale: the variant alternatives are not part of any public discriminator (callers go through pushOwned/pushLazy), so the variant tag was carrying no semantic value — std::any keeps the storage slot flexible without constraining future producer shapes. * refactor(object_store): preserve BufferAnchor end-to-end; restore typed variant Builds on the prior commit's PayloadView-returning pushLazy signature, but fixes two anchor-discarding bugs and restores a typed payload variant. Bugs in the prior implementation: 1. resolveEntry did static_pointer_cast<const vector<uint8_t>>(pv.anchor), forcing every producer's anchor to be exactly a shared_ptr<vector>. Any other anchor type (chunk cache, mmap, parquet column slice) was UB. This negated the entire point of BufferAnchor = shared_ptr<const void>. 2. resolveEntry discarded PayloadView::bytes (the Span) and returned the whole anchor's vector. Producers can no longer publish a sub-range of a larger backing buffer — which is the canonical zero-copy use case (one decompressed MCAP chunk anchored once, many message Spans into it). Both bugs traced to a single root cause: ResolvedObjectEntry::data was still shared_ptr<const vector<uint8_t>>, so resolution had to materialize a vector somehow. Fixed by making ResolvedObjectEntry carry {BufferAnchor anchor, Span<const uint8_t> view} — type-erased anchor + producer-published span. No static_pointer_cast anywhere. Variant + named aliases: ObjectEntry::payload returns to std::variant; the prior std::any traded compile-time exhaustiveness for nothing (still two alternatives, still typed access via the cast). Two named aliases capture the two payload shapes at the type level: using SharedBuffer = std::shared_ptr<const std::vector<uint8_t>>; using LazyCallback = std::function<sdk::PayloadView()>; Trampolines (plugin_data_host.cpp): ObjectBytesBox now holds {BufferAnchor, Span}; toolboxObjectGetBytes returns view.data()/view.size() — no shared_ptr<vector> in the read path. Misc consistency: - pushOwned/pushLazy now return Status (alias for Expected<void, string>); the one other Expected<void, string> use at service_registry_builder.hpp also switched. - PayloadView(shared_ptr<vector>) constructor takes shared_ptr<const vector> to match SharedBuffer and makePayloadView. Tests: - All resolved->data accessors migrated to resolved->view / resolved->anchor across object_store_test, plugin_data_host_object_test, plugin_parser_object_write_test. - Two regression tests added: PushLazyPreservesAnchorType — anchor is shared_ptr<TestBuffer> (not vector). Bug-1 regression: ASAN would catch the prior static cast. PushLazyHonorsSpanSubview — anchor is a 100-byte vector with Span [20,30). Bug-2 regression: verifies view.data()==chunk+20. ./build.sh --debug && ./test.sh → 62/62 passing under ASAN. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(object_store): consolidate ResolvedObjectEntry/ObjectBytesBox on PayloadView ResolvedObjectEntry and ObjectBytesBox both held an inline {anchor, view} pair — the same shape as sdk::PayloadView. Collapse both onto a single PayloadView field; one named point of truth for "bytes + their lifetime anchor" across producer, store, and consumer-handle layers. Also tightens the doc comments touched in the prior commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump version to 0.5.0 Source-incompatible changes in pj_datastore/object_store.hpp: - ObjectEntry::payload variant tag types - pushLazy fetch signature (returns PayloadView) - ResolvedObjectEntry field layout (carries a PayloadView) Per the pre-1.0 convention, this warrants a MINOR bump. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(object_store): sync design doc with PayloadView refactor OBJECT_STORE_DESIGN.md still described the pre-refactor API: a vector-returning lazy callback and a ResolvedObjectEntry carrying a shared_ptr<const vector<uint8_t>> data field. Bring it in line with the code already on this branch: - ObjectEntry::payload is std::variant<SharedBuffer, LazyCallback>; document the two named aliases. - pushLazy's callable returns sdk::PayloadView (Span + type-erased BufferAnchor), enabling zero-copy sub-range views; the store never copies on resolve. - ResolvedObjectEntry carries an sdk::PayloadView; bytes/anchor replace the old data field, and an empty anchor means "no bytes". Docs-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Davide Faconti <davide.faconti@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a8d117d commit 2768118

11 files changed

Lines changed: 163 additions & 57 deletions

File tree

CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ endif()
202202
if(PJ_INSTALL_SDK)
203203
include(CMakePackageConfigHelpers)
204204

205-
set(PJ_PACKAGE_VERSION "0.4.2")
205+
set(PJ_PACKAGE_VERSION "0.5.0")
206206
set(PJ_PACKAGE_CMAKE_DIR ${CMAKE_INSTALL_LIBDIR}/cmake/plotjuggler_core)
207207

208208
install(EXPORT plotjuggler_coreTargets

conanfile.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
plugin_sdk — umbrella for plugin authors (base + dialog SDK + parser SDK)
88
plugin_host — umbrella for host loaders (data_source/parser/toolbox/dialog)
99
10-
A consuming Conan recipe declares e.g. `plotjuggler_core/0.4.2` and then:
10+
A consuming Conan recipe declares e.g. `plotjuggler_core/0.5.0` and then:
1111
1212
find_package(plotjuggler_core REQUIRED COMPONENTS plugin_sdk)
1313
target_link_libraries(my_plugin PRIVATE plotjuggler_core::plugin_sdk)
@@ -27,7 +27,7 @@
2727

2828
class PlotjugglerCoreConan(ConanFile):
2929
name = "plotjuggler_core"
30-
version = "0.4.2"
30+
version = "0.5.0"
3131
# Apache-2.0 covers pj_base + pj_plugins (the plugin-facing SDK);
3232
# MPL-2.0 covers pj_datastore (the storage engine). See LICENSE.
3333
license = "Apache-2.0 AND MPL-2.0"

pj_base/include/pj_base/buffer_anchor.hpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
#include <cstdint>
1717
#include <memory>
18+
#include <utility>
19+
#include <vector>
1820

1921
#include "pj_base/span.hpp"
2022

@@ -44,10 +46,22 @@ struct PayloadView {
4446

4547
PayloadView(Span<const uint8_t> bytes_, BufferAnchor anchor_) : bytes(bytes_), anchor(std::move(anchor_)) {}
4648

47-
PayloadView(std::shared_ptr<std::vector<uint8_t>> buffer)
49+
PayloadView(std::shared_ptr<const std::vector<uint8_t>> buffer)
4850
: bytes(buffer ? Span<const uint8_t>(buffer->data(), buffer->size()) : Span<const uint8_t>()),
4951
anchor(std::move(buffer)) {}
5052
};
5153

54+
/// Wrap a fresh vector as both anchor and Span. Use when the producer has
55+
/// no natural anchor (e.g. raw bytes from a C-ABI fetch); when an upstream
56+
/// allocation already exists, construct PayloadView directly to avoid this
57+
/// helper's copy.
58+
inline PayloadView makePayloadView(std::vector<uint8_t> bytes) {
59+
auto shared = std::make_shared<const std::vector<uint8_t>>(std::move(bytes));
60+
return PayloadView{
61+
Span<const uint8_t>{shared->data(), shared->size()},
62+
BufferAnchor{shared},
63+
};
64+
}
65+
5266
} // namespace sdk
5367
} // namespace PJ

pj_datastore/docs/OBJECT_STORE_DESIGN.md

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,14 @@ struct ObjectTopicDescriptor {
3030
std::string metadata_json;
3131
};
3232

33+
// Eager payload: store-owned bytes, counted against the retention budget.
34+
using SharedBuffer = std::shared_ptr<const std::vector<uint8_t>>;
35+
// Lazy payload: idempotent fetcher returning a view + ownership anchor.
36+
using LazyCallback = std::function<sdk::PayloadView()>;
37+
3338
struct ObjectEntry {
3439
Timestamp timestamp;
35-
std::variant<
36-
std::shared_ptr<const std::vector<uint8_t>>,
37-
std::function<std::vector<uint8_t>()>> payload;
40+
std::variant<SharedBuffer, LazyCallback> payload;
3841
};
3942

4043
struct RetentionBudget {
@@ -55,9 +58,13 @@ order. Equal timestamps are allowed. Out-of-order writes fail.
5558
shared buffer owned by the store. Owned entries contribute to `memoryUsage()`.
5659
5760
`pushLazy(id, timestamp, fetch)` stores a callable instead of bytes. The callable
58-
is invoked on each read and returns a fresh byte vector. Lazy entries do not
59-
contribute to `memoryUsage()` because the store does not retain the bytes between
60-
reads.
61+
is invoked on each read and returns a `sdk::PayloadView` — a `Span<const uint8_t>`
62+
paired with a type-erased `BufferAnchor` that keeps those bytes alive for as long
63+
as the resolved view is held. The producer anchors on whatever already owns the
64+
bytes (a decompressed chunk, an mmap, or a fresh allocation via
65+
`sdk::makePayloadView`), so the store never copies on resolve. Lazy entries do not
66+
contribute to `memoryUsage()` because the store retains the callable, not the
67+
fetched bytes.
6168
6269
Both write paths apply the topic retention budget after the new entry is
6370
inserted.
@@ -80,12 +87,14 @@ Resolved entries contain:
8087
```cpp
8188
struct ResolvedObjectEntry {
8289
Timestamp timestamp;
83-
std::shared_ptr<const std::vector<uint8_t>> data;
90+
sdk::PayloadView payload; // { Span<const uint8_t> bytes; BufferAnchor anchor; }
8491
};
8592
```
8693

87-
The shared pointer keeps resolved bytes alive independently of later store
88-
mutation.
94+
`payload.bytes` is the resolved view; `payload.anchor` keeps those bytes alive
95+
independently of later store mutation (eviction, removal, or `clear()`). For an
96+
owned entry the anchor is the store's `SharedBuffer`; for a lazy entry it is
97+
whatever the fetcher anchored on. An empty `anchor` means "no bytes".
8998

9099
## Retention
91100

pj_datastore/include/pj_datastore/object_store.hpp

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
#include <variant>
1717
#include <vector>
1818

19+
#include "pj_base/buffer_anchor.hpp"
1920
#include "pj_base/expected.hpp"
21+
#include "pj_base/span.hpp"
2022
#include "pj_base/types.hpp"
2123

2224
namespace PJ {
@@ -38,14 +40,21 @@ struct ObjectTopicDescriptor {
3840
std::string metadata_json;
3941
};
4042

43+
/// Eager payload: store-owned bytes, counted against the retention budget.
44+
using SharedBuffer = std::shared_ptr<const std::vector<uint8_t>>;
45+
46+
/// Lazy payload: idempotent, thread-safe fetcher returning bytes + anchor.
47+
/// Invoked on every read; bytes are not counted against the retention budget.
48+
using LazyCallback = std::function<sdk::PayloadView()>;
49+
4150
struct ObjectEntry {
4251
Timestamp timestamp = 0;
43-
std::variant<std::shared_ptr<const std::vector<uint8_t>>, std::function<std::vector<uint8_t>()>> payload;
52+
std::variant<SharedBuffer, LazyCallback> payload;
4453
};
4554

4655
struct ResolvedObjectEntry {
4756
Timestamp timestamp = 0;
48-
std::shared_ptr<const std::vector<uint8_t>> data;
57+
sdk::PayloadView payload;
4958
};
5059

5160
struct RetentionBudget {
@@ -106,10 +115,12 @@ class ObjectStore {
106115

107116
// --- Write ---
108117

109-
Expected<void, std::string> pushOwned(ObjectTopicId id, Timestamp timestamp, std::vector<uint8_t> payload);
118+
Status pushOwned(ObjectTopicId id, Timestamp timestamp, std::vector<uint8_t> payload);
110119

111-
Expected<void, std::string> pushLazy(
112-
ObjectTopicId id, Timestamp timestamp, std::function<std::vector<uint8_t>()> fetch);
120+
// Fetcher runs on every read. Producers anchor on whatever owns the bytes
121+
// (chunk cache, mmap, fresh allocation); the store never copies — it just
122+
// retains the anchor through PayloadView.
123+
Status pushLazy(ObjectTopicId id, Timestamp timestamp, LazyCallback fetch);
113124

114125
// --- Read ---
115126

pj_datastore/src/object_store.cpp

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,7 @@ std::vector<ObjectTopicId> ObjectStore::listTopics(DatasetId dataset_id) const {
6767

6868
// --- Write ---
6969

70-
Expected<void, std::string> ObjectStore::pushOwned(
71-
ObjectTopicId id, Timestamp timestamp, std::vector<uint8_t> payload) {
70+
Status ObjectStore::pushOwned(ObjectTopicId id, Timestamp timestamp, std::vector<uint8_t> payload) {
7271
std::shared_lock store_lock(store_mutex_);
7372
auto* series = findSeries(id);
7473
if (series == nullptr) {
@@ -94,8 +93,7 @@ Expected<void, std::string> ObjectStore::pushOwned(
9493
return {};
9594
}
9695

97-
Expected<void, std::string> ObjectStore::pushLazy(
98-
ObjectTopicId id, Timestamp timestamp, std::function<std::vector<uint8_t>()> fetch) {
96+
Status ObjectStore::pushLazy(ObjectTopicId id, Timestamp timestamp, LazyCallback fetch) {
9997
std::shared_lock store_lock(store_mutex_);
10098
auto* series = findSeries(id);
10199
if (series == nullptr) {
@@ -306,11 +304,15 @@ ResolvedObjectEntry ObjectStore::resolveEntry(const ObjectEntry& entry) {
306304
ResolvedObjectEntry resolved;
307305
resolved.timestamp = entry.timestamp;
308306

309-
if (const auto* owned = std::get_if<std::shared_ptr<const std::vector<uint8_t>>>(&entry.payload)) {
310-
resolved.data = *owned;
311-
} else if (const auto* lazy = std::get_if<std::function<std::vector<uint8_t>()>>(&entry.payload)) {
312-
auto bytes = (*lazy)();
313-
resolved.data = std::make_shared<const std::vector<uint8_t>>(std::move(bytes));
307+
if (const auto* owned = std::get_if<SharedBuffer>(&entry.payload)) {
308+
if (*owned) {
309+
resolved.payload = sdk::PayloadView{
310+
Span<const uint8_t>{(*owned)->data(), (*owned)->size()},
311+
sdk::BufferAnchor{*owned},
312+
};
313+
}
314+
} else if (const auto* lazy = std::get_if<LazyCallback>(&entry.payload)) {
315+
resolved.payload = (*lazy)();
314316
}
315317

316318
return resolved;
@@ -322,7 +324,7 @@ void ObjectStore::evictFront(ObjectSeries& series) {
322324
}
323325

324326
const auto& front = series.entries.front();
325-
if (const auto* owned = std::get_if<std::shared_ptr<const std::vector<uint8_t>>>(&front.payload)) {
327+
if (const auto* owned = std::get_if<SharedBuffer>(&front.payload); owned != nullptr && *owned) {
326328
series.memory_bytes -= (*owned)->size();
327329
}
328330

pj_datastore/src/plugin_data_host.cpp

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1392,7 +1392,9 @@ bool sourceObjectPushLazy(
13921392
// the lambda; destructor runs exactly once when ObjectStore drops the
13931393
// entry (retention, evict, removeTopic, clear, or store teardown).
13941394
auto holder = std::make_shared<PluginFetchCtx>(fetch_fn, fetch_ctx, fetch_ctx_destroy);
1395-
auto closure = [holder]() -> std::vector<uint8_t> { return holder->invoke(); };
1395+
// Plugins return raw bytes via the C ABI; wrap them as a PayloadView whose
1396+
// anchor is a shared_ptr<const vector<uint8_t>>, per the pushLazy contract.
1397+
auto closure = [holder]() -> sdk::PayloadView { return sdk::makePayloadView(holder->invoke()); };
13961398
auto result = impl->store.pushLazy(ObjectTopicId{topic.id}, timestamp_ns, std::move(closure));
13971399
if (!result) {
13981400
impl->setError(result.error());
@@ -1433,10 +1435,10 @@ void sourceObjectSetRetentionBudget(
14331435
// Toolbox object read host trampolines
14341436
// ---------------------------------------------------------------------------
14351437

1436-
/// Box holding the shared_ptr that keeps ObjectStore bytes alive. One
1437-
/// allocated per successful read_latest_at; freed by release_bytes.
1438+
/// Heap holder for the PayloadView backing PJ_object_bytes_handle_t.
1439+
/// Allocated by read_latest_at; freed by release_bytes.
14381440
struct ObjectBytesBox {
1439-
std::shared_ptr<const std::vector<uint8_t>> bytes;
1441+
sdk::PayloadView payload;
14401442
};
14411443

14421444
PJ_object_topic_handle_t toolboxObjectLookupTopic(void* ctx, PJ_string_view_t topic_name) noexcept {
@@ -1506,12 +1508,12 @@ bool toolboxObjectReadLatestAt(
15061508
*out_handle = nullptr;
15071509
try {
15081510
auto entry = impl->store.latestAt(ObjectTopicId{topic.id}, timestamp_ns);
1509-
if (!entry.has_value() || entry->data == nullptr) {
1511+
if (!entry.has_value() || entry->payload.anchor == nullptr) {
15101512
impl->setError("no entry at-or-before timestamp");
15111513
propagateError(out_error, impl->last_error.c_str());
15121514
return false;
15131515
}
1514-
auto* box = new ObjectBytesBox{std::move(entry->data)};
1516+
auto* box = new ObjectBytesBox{std::move(entry->payload)};
15151517
*out_handle = reinterpret_cast<PJ_object_bytes_handle_t>(box);
15161518
if (out_timestamp != nullptr) {
15171519
*out_timestamp = entry->timestamp;
@@ -1540,14 +1542,14 @@ void toolboxObjectGetBytes(PJ_object_bytes_handle_t handle, const uint8_t** out_
15401542
return;
15411543
}
15421544
auto* box = reinterpret_cast<ObjectBytesBox*>(handle);
1543-
if (!box->bytes) {
1545+
if (box->payload.bytes.empty()) {
15441546
return;
15451547
}
15461548
if (out_data != nullptr) {
1547-
*out_data = box->bytes->data();
1549+
*out_data = box->payload.bytes.data();
15481550
}
15491551
if (out_size != nullptr) {
1550-
*out_size = box->bytes->size();
1552+
*out_size = box->payload.bytes.size();
15511553
}
15521554
}
15531555

@@ -1631,7 +1633,7 @@ bool parserObjectPushLazy(
16311633
}
16321634
try {
16331635
auto holder = std::make_shared<PluginFetchCtx>(fetch_fn, fetch_ctx, fetch_ctx_destroy);
1634-
auto closure = [holder]() -> std::vector<uint8_t> { return holder->invoke(); };
1636+
auto closure = [holder]() -> sdk::PayloadView { return sdk::makePayloadView(holder->invoke()); };
16351637
auto result = impl->store.pushLazy(impl->bound_topic, timestamp_ns, std::move(closure));
16361638
if (!result) {
16371639
impl->setError(result.error());

0 commit comments

Comments
 (0)