From 0e2ef14f024687681bcc25c15b0862db214d1333 Mon Sep 17 00:00:00 2001 From: David Schoch Date: Fri, 5 Jun 2026 16:37:50 +0200 Subject: [PATCH 1/3] perf: share the graph weak-reference across constructed sequences Constructing a vertex/edge sequence attached a graph reference per object via add_vses_graph_ref(), which calls .Call(Rx_igraph_copy_env), .Call(Rx_igraph_make_weak_ref) and .Call(Rx_igraph_get_graph_id) for every object. For functions returning many sequences (e.g. max_cliques returning tens of thousands) this dominated construction time -- profiling showed it was ~75% of the cost, far more than name building. The weak reference's key is the graph's environment, which is identical for every sequence of a graph (Rf_duplicate() is a no-op on an environment, so get_vs_ref() returns the same env each call). A single shared weak reference is therefore semantically identical to one per object: while the graph is alive the reference resolves, and once the graph is released the (weak) reference reports it gone -- verified that get_vs_graph() still returns NULL after rm(graph); gc(). simple_es_index() already propagates env/graph from its input, so edge construction only needed the redundant per-object add_vses_graph_ref() call removed. simple_vs_index() now propagates env/graph the same way, and unsafe_create_vs()/unsafe_create_es() rely on that propagation. The existing `lapply(res, unsafe_create_vs, graph = graph, verts = V(graph))` call sites then share the single weak reference built by V()/E() with no call-site or codegen changes. max_cliques(sample_gnp(500, 0.15)) on a named graph: ~338ms -> ~200ms. Co-Authored-By: Claude Opus 4.8 (1M context) --- R/iterators.R | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/R/iterators.R b/R/iterators.R index 95f20d938a9..b8a5704e08c 100644 --- a/R/iterators.R +++ b/R/iterators.R @@ -313,8 +313,9 @@ unsafe_create_vs <- function(graph, idx, verts = NULL) { if (is.null(verts)) { verts <- V(graph) } - res <- simple_vs_index(verts, idx, na_ok = TRUE) - add_vses_graph_ref(res, graph) + # `simple_vs_index()` carries the graph reference over from `verts`, so the + # weak reference built once by `V(graph)` is shared across every call here. + simple_vs_index(verts, idx, na_ok = TRUE) } # Internal function to quickly convert integer vectors to igraph.es @@ -325,8 +326,9 @@ unsafe_create_es <- function(graph, idx, es = NULL) { if (is.null(es)) { es <- E(graph) } - res <- simple_es_index(es, idx, na_ok = TRUE) - add_vses_graph_ref(res, graph) + # `simple_es_index()` already carries the graph reference over from `es`, + # so the weak reference built once by `E(graph)` is shared across calls. + simple_es_index(es, idx, na_ok = TRUE) } @@ -487,6 +489,12 @@ simple_vs_index <- function(x, i, na_ok = FALSE) { if (!na_ok && anyNA(res)) { cli::cli_abort("Unknown vertex selected.") } + # Carry the graph reference over from `x`, mirroring `simple_es_index()`. + # All sequences derived from the same `x` (e.g. a single `V(graph)` reused + # across an lapply()) then share its weak reference instead of each minting + # a fresh one, which is the dominant cost when constructing many sequences. + attr(res, "env") <- attr(x, "env") + attr(res, "graph") <- attr(x, "graph") class(res) <- "igraph.vs" res } From 02dbf5bf81b4666812e1501fbbb663e126d70125 Mon Sep 17 00:00:00 2001 From: David Schoch Date: Fri, 5 Jun 2026 16:43:57 +0200 Subject: [PATCH 2/3] perf: build sequence payload directly and set attributes in one pass Two construction-cost reductions on top of the shared weak reference: * simple_vs_index() now sets names/class/env/graph in a single `attributes<-` call instead of separate `attr<-`/`class<-` assignments. Each incremental assignment shallow-copies the vector, and that copying dominated when building many sequences. * unsafe_create_vs() no longer routes through simple_vs_index(). Its `idx` are vertex IDs from C and `verts` is always the full V(graph), so `verts[idx]` just reproduces `idx`; we now use the IDs directly as the (integer) payload and subset the names off `verts`, avoiding a full copy of V(graph) per object. Payload type (integer), names, NA handling and graph recovery are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) Co-Authored-By: Claude Opus 5 (1M context) --- R/iterators.R | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/R/iterators.R b/R/iterators.R index b8a5704e08c..70781b467ba 100644 --- a/R/iterators.R +++ b/R/iterators.R @@ -313,9 +313,21 @@ unsafe_create_vs <- function(graph, idx, verts = NULL) { if (is.null(verts)) { verts <- V(graph) } - # `simple_vs_index()` carries the graph reference over from `verts`, so the - # weak reference built once by `V(graph)` is shared across every call here. - simple_vs_index(verts, idx, na_ok = TRUE) + # `idx` are vertex IDs straight from C, and `verts` is the full `V(graph)`, + # so `verts[idx]` would just be `idx` again -- skip that copy and use the + # IDs directly as the payload. Names are subset from `verts`, and the graph + # reference is shared from `verts`. All attributes are set in one + # `attributes<-` call to avoid the per-object shallow copies that dominate + # when many sequences are built (e.g. `max_cliques()`). + vertex_names <- attr(verts, "names") + res <- as.integer(idx) + attributes(res) <- list( + names = if (is.null(vertex_names)) NULL else vertex_names[idx], + class = "igraph.vs", + env = attr(verts, "env"), + graph = attr(verts, "graph") + ) + res } # Internal function to quickly convert integer vectors to igraph.es @@ -489,13 +501,19 @@ simple_vs_index <- function(x, i, na_ok = FALSE) { if (!na_ok && anyNA(res)) { cli::cli_abort("Unknown vertex selected.") } - # Carry the graph reference over from `x`, mirroring `simple_es_index()`. - # All sequences derived from the same `x` (e.g. a single `V(graph)` reused - # across an lapply()) then share its weak reference instead of each minting - # a fresh one, which is the dominant cost when constructing many sequences. - attr(res, "env") <- attr(x, "env") - attr(res, "graph") <- attr(x, "graph") - class(res) <- "igraph.vs" + # Set every attribute in a single `attributes<-` call rather than one + # `attr<-`/`class<-` at a time: each incremental assignment shallow-copies + # the vector, and that copying dominates when many sequences are built + # (e.g. `max_cliques()`). `names` is carried over from the subset above; + # env/graph are carried from `x`, mirroring `simple_es_index()`, so + # sequences derived from one `V(graph)` share its weak reference instead of + # each minting a fresh one. + attributes(res) <- list( + names = attr(res, "names"), + class = "igraph.vs", + env = attr(x, "env"), + graph = attr(x, "graph") + ) res } From 1220792a946cbab2d08f593b7a6cb27b58356063 Mon Sep 17 00:00:00 2001 From: David Schoch Date: Fri, 4 Sep 2026 13:05:38 +0200 Subject: [PATCH 3/3] test(touchstone): benchmark vertex/edge sequence construction Add a benchmark group exercising the sequence-construction path on named graphs, where building the `names`/`vnames` attribute and attaching the graph reference dominate: max_cliques (thousands of vertex sequences), head_of over every edge, and V()/E() on a large named graph. Repeat counts follow the convention documented at the top of the script: warm up in the setup block, then loop a literal number of times so the measured region lands near 100 ms on the base branch. Co-Authored-By: Claude Opus 4.8 (1M context) Co-Authored-By: Claude Opus 5 (1M context) --- touchstone/script.R | 77 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/touchstone/script.R b/touchstone/script.R index fce025cd817..cc4900e25ac 100644 --- a/touchstone/script.R +++ b/touchstone/script.R @@ -254,5 +254,82 @@ benchmark_run( n = 20 ) +# --------------------------------------------------------------------------- +# Group #5 - vertex/edge sequence construction on named graphs +# Functions that return (many) vertex/edge sequences pay for building the +# `names`/`vnames` attribute and attaching a graph reference to every object. +# These benchmarks exercise that construction path on *named* graphs, where +# the cost is highest. `max_cliques()` is the canonical case: it returns tens +# of thousands of vertex sequences, one per clique. +# --------------------------------------------------------------------------- +benchmark_run( + expr_before_benchmark = { + library(igraph) + set.seed(42) + g <- sample_gnp(200L, 0.16, directed = FALSE) + V(g)$name <- paste0("v", seq_len(gorder(g))) + for (i in 1:2) { + max_cliques(g) + } + gc(full = TRUE) + }, + max_cliques_named = for (i in 1:4) { + max_cliques(g) + }, + n = 20 +) + +benchmark_run( + expr_before_benchmark = { + library(igraph) + set.seed(42) + g <- sample_gnm(1000L, 5000L) + V(g)$name <- paste0("v", seq_len(1000L)) + es <- E(g) + for (i in 1:5) { + head_of(g, es) + } + gc(full = TRUE) + }, + head_of_named = for (i in 1:320) { + head_of(g, es) + }, + n = 20 +) + +benchmark_run( + expr_before_benchmark = { + library(igraph) + set.seed(42) + g <- sample_gnm(20000L, 50000L) + V(g)$name <- paste0("v", seq_len(20000L)) + for (i in 1:5) { + V(g) + } + gc(full = TRUE) + }, + V_named = for (i in 1:2700) { + V(g) + }, + n = 20 +) + +benchmark_run( + expr_before_benchmark = { + library(igraph) + set.seed(42) + g <- sample_gnm(20000L, 50000L) + V(g)$name <- paste0("v", seq_len(20000L)) + for (i in 1:2) { + E(g) + } + gc(full = TRUE) + }, + E_named = for (i in 1:15) { + E(g) + }, + n = 20 +) + # Create the artifacts consumed by the GitHub Action. benchmark_analyze()