Skip to content

update from review #128

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.

Sign up for GitHub

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

Open
MiXaiLL76 wants to merge 2 commits into
base: perf/cpp_cache
Choose a base branch
from
Open

update from review #128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b664043
update from review
MiXaiLL76 Aug 29, 2026
19d21db
Update test command to include additional test case
Borda Sep 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 35 additions & 21 deletions csrc/faster_eval_api/coco_eval/dataset.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#include <algorithm>
#include <cstdint>
#include <iterator>
#include <memory>
#include <numeric>

// clang-format off
Expand Down Expand Up @@ -65,8 +67,8 @@ py::tuple LightweightDataset::make_tuple() const {
// Create a list of (img_id, cat_id, annotation_list) tuples
py::list serialized_data;
for (const auto& kv : annotations) {
auto key = kv.first;
auto ann_list = kv.second;
const auto& key = kv.first;
const auto& ann_list = kv.second;

py::list py_ann_list;
for (const auto& ann : ann_list) {
Expand Down Expand Up @@ -218,19 +220,24 @@ std::vector<InstanceAnnotation> LightweightDataset::get_cpp_annotations(
for (;;) {
std::vector<py::object> annotations;
uint64_t snapshot_version = 0;
CppCacheEntry cached;
{
std::lock_guard<std::mutex> guard(mutex);
auto cache_it = cpp_cache.find(key);
if (cache_it != cpp_cache.end()) {
return cache_it->second;
cached = cache_it->second;
} else {
auto annotation_it = annotation_refs.find(key);
if (annotation_it == annotation_refs.end()) {
return {};
}
annotations = annotation_it->second;
snapshot_version = annotation_version;
}

auto annotation_it = annotation_refs.find(key);
if (annotation_it == annotation_refs.end()) {
return {};
}
annotations = annotation_it->second;
snapshot_version = annotation_version;
}
if (cached) {
// Clone the payload after releasing the mutex.
return *cached;
}

std::vector<InstanceAnnotation> parsed_annotations;
Expand All @@ -242,15 +249,20 @@ std::vector<InstanceAnnotation> LightweightDataset::get_cpp_annotations(

// Conversion can run arbitrary Python hooks. Only publish its
// result when no writer has replaced its source annotations.
std::lock_guard<std::mutex> guard(mutex);
auto cache_it = cpp_cache.find(key);
if (cache_it != cpp_cache.end()) {
return cache_it->second;
{
std::lock_guard<std::mutex> guard(mutex);
auto cache_it = cpp_cache.find(key);
if (cache_it != cpp_cache.end()) {
cached = cache_it->second;
} else if (snapshot_version == annotation_version) {
cached = std::make_shared<
const std::vector<InstanceAnnotation>>(
std::move(parsed_annotations));
cpp_cache.emplace(key, cached);
}
}
if (snapshot_version == annotation_version) {
auto inserted = cpp_cache.emplace(
key, std::move(parsed_annotations));
return inserted.first->second;
if (cached) {
return *cached;
}
}
}
Expand Down Expand Up @@ -293,10 +305,12 @@ LightweightDataset::get_cpp_instances(const std::vector<double>& img_ids,
for (size_t j = 0; j < cat_ids.size(); ++j) {
int64_t cat_id =
static_cast<int64_t>(cat_ids[j]);
const std::vector<InstanceAnnotation> anns =
std::vector<InstanceAnnotation> anns =
get_cpp_annotations(img_id, cat_id);
merged.insert(merged.end(), anns.begin(),
anns.end());
merged.insert(
merged.end(),
std::make_move_iterator(anns.begin()),
std::make_move_iterator(anns.end()));
}
// Wrap merged vector in an outer vector for consistency
result.emplace_back(1, std::move(merged));
Expand Down
18 changes: 13 additions & 5 deletions csrc/faster_eval_api/coco_eval/dataset.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <pybind11/pybind11.h>

#include <cstdint>
#include <memory>
#include <mutex>
#include <unordered_map>
#include <vector>
Expand Down Expand Up @@ -38,12 +39,14 @@ class LightweightDataset {

// A std::mutex member is neither copyable nor movable, which would
// otherwise delete these implicitly and break the pickle __setstate__
// factory that returns a dataset by value. Move the containers and
// leave each object with its own fresh mutex; the source is locked so
// the move cannot race a concurrent reader.
// factory that returns a dataset by value. Only move *construction* is
// supported (all pybind needs): move the containers and leave each
// object with its own fresh mutex; the source is locked so the move
// cannot race a concurrent reader. Assignment is not supported.
LightweightDataset(LightweightDataset&& other) noexcept;
LightweightDataset(const LightweightDataset&) = delete;
LightweightDataset& operator=(const LightweightDataset&) = delete;
LightweightDataset& operator=(LightweightDataset&&) = delete;

// Store reference to annotation instead of copying data
void append_ref(double img_id, double cat_id, py::object ann_ref);
Expand Down Expand Up @@ -97,9 +100,14 @@ class LightweightDataset {
using AnnotationRefs =
std::unordered_map<AnnotationKey, std::vector<py::object>,
hash_pair>;
// Cached entries are held through a shared_ptr to an immutable vector
// so a reader can copy the (cheap) handle under the mutex and clone the
// payload only after releasing it; a concurrent clear_cache_entry then
// cannot leave a dangling reference either.
using CppCacheEntry =
std::shared_ptr<const std::vector<InstanceAnnotation>>;
using CppCache =
std::unordered_map<AnnotationKey, std::vector<InstanceAnnotation>,
hash_pair>;
std::unordered_map<AnnotationKey, CppCacheEntry, hash_pair>;

// Lightweight storage: only references to Python objects
AnnotationRefs annotation_refs;
Expand Down
3 changes: 2 additions & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ requires = [
"numpy>=2.0.0rc1",
"Cython",
# 3.0.0 is the first release supporting CPython 3.14 and 3.14t.
"pybind11>=3.0.0, <4",
"pybind11>=2.13.0; python_version<'3.14'",
"pybind11>=3.0.0; python_version>='3.14'"
]
build-backend = "setuptools.build_meta"

Expand Down
Loading