Skip to content

Commit 5c3ffb7

Browse files
haochengxiahaochengxia
committed
Fix comments by copilot
1 parent 9d0b8a7 commit 5c3ffb7

3 files changed

Lines changed: 106 additions & 47 deletions

File tree

‎libCacheSim-python/libcachesim/synthetic_reader.py‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ def read_one_req(self, req: Request) -> Request:
9595
req.obj_id = obj_id
9696
req.obj_size = self.obj_size
9797
req.clock_time = self.current_pos * self.time_span // self.num_of_req
98-
req.op = ReqOp.OP_NOP
98+
req.op = ReqOp.OP_READ
9999
req.valid = True
100100

101101
self.current_pos += 1
@@ -132,7 +132,7 @@ def read_first_req(self, req: Request) -> Request:
132132
req.obj_id = obj_id
133133
req.obj_size = self.obj_size
134134
req.clock_time = 0
135-
req.op = ReqOp.OP_NOP
135+
req.op = ReqOp.OP_READ
136136
req.valid = True
137137
return req
138138

@@ -146,7 +146,7 @@ def read_last_req(self, req: Request) -> Request:
146146
req.obj_id = obj_id
147147
req.obj_size = self.obj_size
148148
req.clock_time = (self.num_of_req - 1) * self.time_span // self.num_of_req
149-
req.op = ReqOp.OP_NOP
149+
req.op = ReqOp.OP_READ
150150
req.valid = True
151151
return req
152152

@@ -165,7 +165,7 @@ def read_one_req_above(self, req: Request) -> Request:
165165
req.obj_id = obj_id
166166
req.obj_size = self.obj_size
167167
req.clock_time = (self.current_pos + 1) * self.time_span // self.num_of_req
168-
req.op = ReqOp.OP_NOP
168+
req.op = ReqOp.OP_READ
169169
req.valid = True
170170
return req
171171

@@ -207,7 +207,7 @@ def __getitem__(self, index: int) -> Request:
207207
req.obj_id = obj_id
208208
req.obj_size = self.obj_size
209209
req.clock_time = index * self.time_span // self.num_of_req
210-
req.op = ReqOp.OP_NOP
210+
req.op = ReqOp.OP_READ
211211
req.valid = True
212212
return req
213213

@@ -256,7 +256,8 @@ def _gen_uniform(m: int, n: int, start: int = 0) -> np.ndarray:
256256
"""
257257
if m <= 0 or n <= 0:
258258
raise ValueError("num_objects and num_requests must be positive")
259-
return np.random.randint(0, m, n) + start
259+
# Optimized: directly generate in the target range for better performance
260+
return np.random.randint(start, start + m, n)
260261

261262

262263
class _BaseRequestGenerator:
@@ -302,7 +303,7 @@ def __iter__(self) -> Iterator[Request]:
302303
req.clock_time = i * self.time_span // self.num_requests
303304
req.obj_id = obj_id
304305
req.obj_size = self.obj_size
305-
req.op = ReqOp.OP_NOP
306+
req.op = ReqOp.OP_READ
306307
req.valid = True
307308
yield req
308309

‎libCacheSim-python/src/export_cache.cpp‎

Lines changed: 82 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include <iostream>
1414
#include <sstream>
15+
#include <memory>
1516

1617
#include "config.h"
1718
#include "dataStructure/hashtable/hashtable.h"
@@ -58,7 +59,10 @@ struct RequestDeleter {
5859
// **** Python plugin cache implementation BEGIN ****
5960
// ***********************************************************************
6061

61-
typedef struct pypluginCache_params {
62+
// Forward declaration with appropriate visibility
63+
struct pypluginCache_params;
64+
65+
typedef struct __attribute__((visibility("hidden"))) pypluginCache_params {
6266
py::object data; ///< Plugin's internal data structure (python object)
6367
py::function cache_init_hook;
6468
py::function cache_hit_hook;
@@ -69,6 +73,23 @@ typedef struct pypluginCache_params {
6973
std::string cache_name;
7074
} pypluginCache_params_t;
7175

76+
// Custom deleter for pypluginCache_params_t
77+
struct PypluginCacheParamsDeleter {
78+
void operator()(pypluginCache_params_t* ptr) const {
79+
if (ptr != nullptr) {
80+
// Call the free hook if available before deletion
81+
if (!ptr->cache_free_hook.is_none()) {
82+
try {
83+
ptr->cache_free_hook(ptr->data);
84+
} catch (...) {
85+
// Ignore exceptions during cleanup to prevent double-fault
86+
}
87+
}
88+
delete ptr;
89+
}
90+
}
91+
};
92+
7293
static void pypluginCache_free(cache_t* cache);
7394
static bool pypluginCache_get(cache_t* cache, const request_t* req);
7495
static cache_obj_t* pypluginCache_find(cache_t* cache, const request_t* req,
@@ -84,47 +105,70 @@ cache_t* pypluginCache_init(
84105
py::function cache_init_hook, py::function cache_hit_hook,
85106
py::function cache_miss_hook, py::function cache_eviction_hook,
86107
py::function cache_remove_hook, py::function cache_free_hook) {
87-
// Initialize base cache structure
88-
cache_t* cache = cache_struct_init(cache_name.c_str(), ccache_params, NULL);
89-
90-
// Set function pointers for cache operations
91-
cache->cache_init = NULL;
92-
cache->cache_free = pypluginCache_free;
93-
cache->get = pypluginCache_get;
94-
cache->find = pypluginCache_find;
95-
cache->insert = pypluginCache_insert;
96-
cache->evict = pypluginCache_evict;
97-
cache->remove = pypluginCache_remove;
98-
cache->to_evict = pypluginCache_to_evict;
99-
cache->get_occupied_byte = cache_get_occupied_byte_default;
100-
cache->get_n_obj = cache_get_n_obj_default;
101-
cache->can_insert = cache_can_insert_default;
102-
cache->obj_md_size = 0;
103-
104-
// Allocate and initialize plugin parameters
105-
pypluginCache_params_t* params = new pypluginCache_params_t();
106-
params->cache_name = cache_name;
107-
params->cache_init_hook = cache_init_hook;
108-
params->cache_hit_hook = cache_hit_hook;
109-
params->cache_miss_hook = cache_miss_hook;
110-
params->cache_eviction_hook = cache_eviction_hook;
111-
params->cache_remove_hook = cache_remove_hook;
112-
params->cache_free_hook = cache_free_hook;
113-
params->data = cache_init_hook(ccache_params);
114-
115-
cache->eviction_params = params;
116-
117-
return cache;
108+
109+
// Initialize base cache structure with exception safety
110+
cache_t* cache = nullptr;
111+
std::unique_ptr<pypluginCache_params_t, PypluginCacheParamsDeleter> params;
112+
113+
try {
114+
cache = cache_struct_init(cache_name.c_str(), ccache_params, NULL);
115+
if (!cache) {
116+
throw std::runtime_error("Failed to initialize cache structure");
117+
}
118+
119+
// Set function pointers for cache operations
120+
cache->cache_init = NULL;
121+
cache->cache_free = pypluginCache_free;
122+
cache->get = pypluginCache_get;
123+
cache->find = pypluginCache_find;
124+
cache->insert = pypluginCache_insert;
125+
cache->evict = pypluginCache_evict;
126+
cache->remove = pypluginCache_remove;
127+
cache->to_evict = pypluginCache_to_evict;
128+
cache->get_occupied_byte = cache_get_occupied_byte_default;
129+
cache->get_n_obj = cache_get_n_obj_default;
130+
cache->can_insert = cache_can_insert_default;
131+
cache->obj_md_size = 0;
132+
133+
// Allocate and initialize plugin parameters using smart pointer with custom deleter
134+
params = std::unique_ptr<pypluginCache_params_t, PypluginCacheParamsDeleter>(
135+
new pypluginCache_params_t(), PypluginCacheParamsDeleter());
136+
params->cache_name = cache_name;
137+
params->cache_init_hook = cache_init_hook;
138+
params->cache_hit_hook = cache_hit_hook;
139+
params->cache_miss_hook = cache_miss_hook;
140+
params->cache_eviction_hook = cache_eviction_hook;
141+
params->cache_remove_hook = cache_remove_hook;
142+
params->cache_free_hook = cache_free_hook;
143+
144+
// Initialize the cache data - this might throw
145+
params->data = cache_init_hook(ccache_params);
146+
147+
// Transfer ownership to the cache structure
148+
cache->eviction_params = params.release();
149+
150+
return cache;
151+
152+
} catch (...) {
153+
// Clean up on exception
154+
if (cache) {
155+
cache_struct_free(cache);
156+
}
157+
// params will be automatically cleaned up by smart pointer destructor
158+
throw; // Re-throw the exception
159+
}
118160
}
119161

120162
static void pypluginCache_free(cache_t* cache) {
121-
pypluginCache_params_t* params =
122-
(pypluginCache_params_t*)cache->eviction_params;
123-
124-
if (!params->cache_free_hook.is_none()) {
125-
params->cache_free_hook(params->data);
163+
if (!cache || !cache->eviction_params) {
164+
return;
126165
}
127-
delete params;
166+
167+
// Use smart pointer for automatic cleanup
168+
std::unique_ptr<pypluginCache_params_t, PypluginCacheParamsDeleter> params(
169+
static_cast<pypluginCache_params_t*>(cache->eviction_params));
170+
171+
// The smart pointer destructor will handle cleanup automatically
128172
cache_struct_free(cache);
129173
}
130174

‎libCacheSim-python/src/export_reader.cpp‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,14 @@ struct RequestDeleter {
4242

4343
struct ReaderInitParamDeleter {
4444
void operator()(reader_init_param_t* ptr) const {
45-
if (ptr != nullptr) free(ptr);
45+
if (ptr != nullptr) {
46+
// Free the strdup'ed string if it exists
47+
if (ptr->binary_fmt_str != nullptr) {
48+
free(ptr->binary_fmt_str);
49+
ptr->binary_fmt_str = nullptr;
50+
}
51+
free(ptr);
52+
}
4653
}
4754
};
4855

@@ -123,9 +130,16 @@ void export_reader(py::module& m) {
123130
const std::string& delimiter, ssize_t trace_start_offset,
124131
sampler_t* sampler) {
125132
reader_init_param_t params = default_reader_init_params();
133+
134+
// Safe string handling with proper error checking
126135
if (!binary_fmt_str.empty()) {
127-
params.binary_fmt_str = strdup(binary_fmt_str.c_str());
136+
char* fmt_str = strdup(binary_fmt_str.c_str());
137+
if (!fmt_str) {
138+
throw std::bad_alloc();
139+
}
140+
params.binary_fmt_str = fmt_str;
128141
}
142+
129143
params.ignore_obj_size = ignore_obj_size;
130144
params.ignore_size_zero_req = ignore_size_zero_req;
131145
params.obj_id_is_num = obj_id_is_num;

0 commit comments

Comments
 (0)