Skip to content

Commit befefda

Browse files
committed
Refactor is_matched_graph for better maintainability
** Description of the problem ** `is_matched_graph` implementation is a bit confusing because is not so easy to understand the comparison between the current graph and the properties of the cached graphs. The usage of `continue` to skip graphs of different sizes, and a combination of a `break` and a flag to exit the inner loop to simulate a `continue` in the outer loop, make it very difficult to follow the logic. Moreover, the passed arguments and the function name do not match the intended semantics properly. ** Proposed solution ** * Rename the function to `is_matched_graph_in_cache` to better reflect its intention. * Pass only the graph cache list instead of the whole context. * Enclose the graph node traversal check inside a lambda to avoid having to use statements that break the control flow arbitrarily. * Rename variables in a more meaningful way. * Update the corresponding function call.
1 parent 3659aa2 commit befefda

File tree

1 file changed

+26
-18
lines changed

1 file changed

+26
-18
lines changed

ggml/src/ggml-cann/ggml-cann.cpp

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2199,34 +2199,42 @@ static bool ggml_graph_node_has_matching_properties(ggml_tensor *
21992199
* function returns true. Otherwise, the function returns false, indicating that a new
22002200
* CANN graph needs to be captured.
22012201
*
2202-
* @param cann_ctx The CANN backend context containing the graph cache.
2203-
* @param cgraph The current ggml computation graph.
2202+
* @param graph_cache The graph cache.
2203+
* @param cgraph The current ggml computation graph.
22042204
* @return true if a matching cached graph exists; false otherwise.
22052205
*/
2206-
static bool is_matched_graph(ggml_backend_cann_context * cann_ctx, ggml_cgraph * cgraph) {
2207-
ggml_cann_graph_lru_cache & lru_cache = cann_ctx->graph_lru_cache;
2208-
for (auto & graph_ptr : lru_cache.cache_list) {
2209-
// Skip graphs with a different number of nodes.
2210-
if (graph_ptr->ggml_graph_properties.size() != static_cast<size_t>(cgraph->n_nodes)) {
2211-
continue;
2206+
static bool is_matched_graph_in_cache(ggml_cann_graph_lru_cache & graph_cache, ggml_cgraph * cgraph) {
2207+
// lambda for checking if all nodes of the current graph match some given properties.
2208+
auto nodes_match_properties = [& cgraph](auto & properties) {
2209+
auto n_nodes = cgraph->n_nodes;
2210+
2211+
// Reject if the list sizes do not match.
2212+
if (properties.size() != static_cast<size_t>(n_nodes)) {
2213+
return false;
22122214
}
22132215

2214-
// Check if all nodes match.
2215-
bool all_match = true;
2216-
for (int i = 0; i < cgraph->n_nodes; ++i) {
2217-
if (!ggml_graph_node_has_matching_properties(cgraph->nodes[i], &graph_ptr->ggml_graph_properties[i])) {
2218-
all_match = false;
2219-
break;
2216+
// Check all nodes of the graph.
2217+
for (int i = 0; i < n_nodes; ++i) {
2218+
// Reject if a node does not match its expected properties.
2219+
if (!ggml_graph_node_has_matching_properties(cgraph->nodes[i], & properties[i])) {
2220+
return false;
22202221
}
22212222
}
22222223

2223-
if (all_match) {
2224-
// update cache_list && renturn graph_ptr
2225-
lru_cache.move_to_front(graph_ptr);
2224+
// All nodes matched its expected properties.
2225+
return true;
2226+
};
2227+
2228+
// Search for a matching graph in the cache.
2229+
for (auto & cached_graph_ptr : graph_cache.cache_list) {
2230+
if (nodes_match_properties(cached_graph_ptr->ggml_graph_properties)) {
2231+
// A matching graph was found.
2232+
graph_cache.move_to_front(cached_graph_ptr);
22262233
return true;
22272234
}
22282235
}
22292236

2237+
// A matching graph was not found.
22302238
return false;
22312239
}
22322240
#endif // USE_ACL_GRAPH
@@ -2331,7 +2339,7 @@ static enum ggml_status ggml_backend_cann_graph_compute(ggml_backend_t backend,
23312339

23322340
if (use_cann_graph) {
23332341
// If no matching graph is found, the graph needs to be recaptured.
2334-
cann_graph_update_required = !is_matched_graph(cann_ctx, cgraph);
2342+
cann_graph_update_required = !is_matched_graph_in_cache(cann_ctx->graph_lru_cache, cgraph);
23352343
if (cann_graph_update_required) {
23362344
// If no matching graph is found, add a new ACL graph.
23372345
add_lru_matched_graph_node_properties(cann_ctx, cgraph);

0 commit comments

Comments
 (0)