Skip to content

Commit 04fe32b

Browse files
phoebusmphoebusm
committed
Better chart and naming
1 parent 696dc62 commit 04fe32b

File tree

3 files changed

+43
-45
lines changed

3 files changed

+43
-45
lines changed

‎cpp/arcticdb/toolbox/python_bindings.cpp

+1-17
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,6 @@ void register_bindings(py::module &m, py::exception<arcticdb::ArcticException>&
153153

154154

155155
using namespace arcticdb::util::query_stats;
156-
157-
// Create QueryStats submodule
158156
auto query_stats_module = tools.def_submodule("QueryStats", "Stats query functionality");
159157
py::enum_<StatsGroupName>(query_stats_module, "StatsGroupName")
160158
.value("arcticdb_call", StatsGroupName::arcticdb_call)
@@ -168,24 +166,19 @@ void register_bindings(py::module &m, py::exception<arcticdb::ArcticException>&
168166
.value("count", StatsName::count)
169167
.export_values();
170168

171-
// Expose StatsGroupLayer class directly
172169
py::class_<StatsGroupLayer, std::shared_ptr<StatsGroupLayer>>(query_stats_module, "StatsGroupLayer")
173170
.def(py::init<>())
174171
.def_readwrite("stats", &StatsGroupLayer::stats_)
175172
.def_property_readonly("next_layer_maps", [](const StatsGroupLayer& self) {
176173
return self.next_layer_maps_;
177174
});
178175

179-
// Expose QueryStats class
180176
query_stats_module.def("current_layer", []() {
181177
return QueryStats::instance().current_layer();
182178
});
183-
184-
// This is what we'll use instead of get_stats_as_dict
185179
query_stats_module.def("root_layers", []() {
186180
return QueryStats::instance().root_layers();
187181
});
188-
189182
query_stats_module.def("is_root_layer_set", []() {
190183
return QueryStats::instance().is_root_layer_set();
191184
});
@@ -194,9 +187,7 @@ void register_bindings(py::module &m, py::exception<arcticdb::ArcticException>&
194187
});
195188
query_stats_module.def("merge_layers", []() {
196189
QueryStats::instance().merge_layers();
197-
});
198-
199-
// Enable/disable QueryStats
190+
});
200191
query_stats_module.def("enable", []() {
201192
QueryStats::instance().is_enabled_ = true;
202193
});
@@ -206,12 +197,5 @@ void register_bindings(py::module &m, py::exception<arcticdb::ArcticException>&
206197
query_stats_module.def("is_enabled", []() {
207198
return QueryStats::instance().is_enabled_;
208199
});
209-
210-
// Previous commented-out bindings were:
211-
// query_stats_module.def("register_new_query_stat_tool", []() {QueryStats::instance().register_new_query_stat_tool(); });
212-
// query_stats_module.def("deregister_query_stat_tool", []() { QueryStats::instance().deregister_query_stat_tool(); });
213-
// query_stats_module.def("is_enabled", []() { return QueryStats::instance().is_enabled(); });
214-
// query_stats_module.def("reset", []() { QueryStats::instance().reset_stats(); });
215-
// query_stats_module.def("get_stats", []() { return QueryStats::instance().get_stats();});
216200
}
217201
} // namespace arcticdb::toolbox::apy

‎cpp/arcticdb/util/query_stats.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ QueryStats& QueryStats::instance() {
5555
}
5656

5757
void QueryStats::merge_layers() {
58-
for (auto& [parent_layer, child_layer] : thread_local_var_.child_layers_) {
59-
parent_layer->merge_from(*child_layer);
58+
for (auto& child_layer : thread_local_var_.child_layers_) {
59+
child_layer.parent_layer_->merge_from(*child_layer.root_layer_);
6060
}
6161
thread_local_var_.child_layers_.clear();
6262
}

‎cpp/arcticdb/util/query_stats.hpp

+40-26
Original file line numberDiff line numberDiff line change
@@ -10,30 +10,42 @@
1010
/*
1111
* Class Structure Diagram:
1212
*
13-
* +------------------+
14-
* | QueryStats |
15-
* Temp. storing folly thread root +------------------+
16-
* layer. Will be aggregated to <...| - child_layers_[]| +---------------------+
17-
* root_layer when func. is finished | - root_layer_ ---|------------------->| StatsGroupLayer |
18-
* | - is_enabled_ | +---------------------+
19-
* Ref. pointer to the layer <...| - current_layer_ | | - stats_[] | .......> Storing non-groupable stats
20-
* in use +------------------+ | - next_layer_maps_[]|
21-
* ^ +---------------------+
22-
* | |
23-
* | |
24-
* | |
25-
* | Extend the chain |
26-
* | and temporarily update |
27-
* +-------+-------+ QueryStats's current_layer_ |
28-
* | StatsGroup |................................|
29-
* +---------------+ v
13+
* +--------------------+
14+
* | QueryStats |
15+
* +--------------------+
16+
* | - is_enabled_ |
17+
* | - root_layers_[] |
18+
* | - thread_local_var_|
19+
* +------|-------------+
20+
* |
21+
* v
22+
* +-------------------------+ +---------------------+
23+
* | ThreadLocalQueryStatsVar| | StatsGroupLayer |
24+
* +-------------------------+ +---------------------+
25+
* Temp. storing folly thread<-| - child_layers_[] | | - stats_[] | --> Storing non-groupable stats
26+
* layer. Will be aggregated | - root_layer_ | | - next_layer_maps_[]|
27+
* to root_layer when | - current_layer_ -------|------------------>+---------------------+
28+
* function is finished +-------------------------+ |
29+
* | |
30+
* | Extend the chain |
31+
* | and temporarily update |
32+
* v current_layer_ |
33+
* +---------------+ |
34+
* | StatsGroup |-------------------------------|
35+
* +---------------+ v
3036
* | - prev_layer_ | +-------------------+
31-
* | - start_ | | StatsGroupLayer |-----> ... (more layers)
32-
* | - log_time_ | +-------------------+
33-
* +---------------+
34-
*
37+
* | - start_ | | StatsGroupLayer |
38+
* | | +-------------------+
39+
* +---------------+ |
40+
* v
41+
* .. (more layers)
42+
*
43+
*
44+
*
45+
*
3546
* Structure:
3647
* - QueryStats: Singleton manager class holding the stats collection framework
48+
* - ThreadLocalQueryStatsVar: Thread-local storage for stats layers and management
3749
* - StatsGroupLayer: Hierarchical node containing stats and references to child layers
3850
* - StatsGroup: RAII wrapper that temporarily extends the layer chain during its lifetime
3951
* When created, it adds a new layer and when destroyed, it restores the previous layer state
@@ -61,10 +73,6 @@
6173
#include <fmt/format.h>
6274

6375
namespace arcticdb::util::query_stats {
64-
using StatsGroups = std::vector<std::shared_ptr<std::pair<std::string, std::string>>>;
65-
66-
67-
// process-global stats entry list
6876
enum class StatsGroupName : size_t {
6977
arcticdb_call = 0,
7078
key_type = 1,
@@ -85,9 +93,15 @@ class StatsGroupLayer {
8593
void merge_from(const StatsGroupLayer& other);
8694
};
8795

96+
struct ChildLayer{
97+
std::shared_ptr<StatsGroupLayer> parent_layer_;
98+
std::shared_ptr<StatsGroupLayer> root_layer_;
99+
};
100+
101+
// Collection of thread-local variables that reside in class QueryStats simply for the sake of convenience
88102
struct ThreadLocalQueryStatsVar {
89103
std::mutex child_layer_creation_mutex_;
90-
std::vector<std::pair<std::shared_ptr<StatsGroupLayer>, std::shared_ptr<StatsGroupLayer>>> child_layers_;
104+
std::vector<ChildLayer> child_layers_;
91105
std::shared_ptr<StatsGroupLayer> root_layer_ = nullptr;
92106
std::shared_ptr<StatsGroupLayer> current_layer_ = nullptr;
93107
};

0 commit comments

Comments
 (0)