Why does `rlang_env_dots_list()` clone?

[DavisVaughan]

i.e. why was a clone added here a8dea54?

Shouldn't it be up to the caller to decide whether or not to clone?

We use this in vec_c() to capture ..., and in particular with the vec_c(!!!x) case I was imagining that we could capture x without requiring any cloning of the list.

It would be nice if we could say vec_c(!!!x) was identical to list_unchop(x) with no indices.

library(vctrs)

values <- rep(list(1), 1e6)

bench::mark(
  vec_c(!!!values),
  list_unchop(values)
)
#> Warning: Some expressions had a GC in every iteration; so filtering is
#> disabled.
#> # A tibble: 2 × 6
#>   expression               min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr>          <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 vec_c(!!!values)       151ms    169ms      5.99    19.1MB     18.0
#> 2 list_unchop(values)    143ms    144ms      6.92    11.4MB     19.0

profmem::profmem(vec_c(!!!values), threshold = 1000)
#> Rprofmem memory profiling of:
#> vec_c(!!!values)
#> 
#> Memory allocations (>= 1000 bytes):
#> Number of 'new page' entries not displayed: 5328
#>        what    bytes      calls
#> 1     alloc  8000048    vec_c()
#> 2     alloc  4000048    vec_c()
#> 3     alloc  8000048    vec_c()
#> total       20004512

profmem::profmem(list_unchop(values), threshold = 1000)
#> Rprofmem memory profiling of:
#> list_unchop(values)
#> 
#> Memory allocations (>= 1000 bytes):
#> Number of 'new page' entries not displayed: 1391
#>        what    bytes         calls
#> 1     alloc  4000048 list_unchop()
#> 2     alloc  8000048 list_unchop()
#> total       12000096

^{Created on 2025-08-28 with reprex v2.1.1}

[DavisVaughan]

I can confirm that if you:

• Comment out that clone
• Install dev rlang
• Load CRAN vctrs

Then vec_c(!!!x) is identical to list_unchop(x), which is what I'm hoping for

library(vctrs)

values <- rep(list(1), 1e6)

bench::mark(
  vec_c(!!!values),
  list_unchop(values), 
  iterations = 100
)
#> Warning: Some expressions had a GC in every iteration; so filtering is
#> disabled.
#> # A tibble: 2 × 6
#>   expression               min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr>          <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 vec_c(!!!values)       140ms    144ms      6.77    11.4MB     18.9
#> 2 list_unchop(values)    139ms    143ms      6.86    11.4MB     19.1

[DavisVaughan]

And it feels like this should be possible because list2(!!!x) avoids a clone

> .Internal(inspect(list2(!!!values)))
@1414d0000 19 VECSXP g1c7 [MARK,REF(65535)] (len=1000000, tl=0)
  @13ef6a238 14 REALSXP g1c1 [MARK,REF(65535)] (len=1, tl=0) 1
  @13ef6a238 14 REALSXP g1c1 [MARK,REF(65535)] (len=1, tl=0) 1
  @13ef6a238 14 REALSXP g1c1 [MARK,REF(65535)] (len=1, tl=0) 1
  @13ef6a238 14 REALSXP g1c1 [MARK,REF(65535)] (len=1, tl=0) 1
  @13ef6a238 14 REALSXP g1c1 [MARK,REF(65535)] (len=1, tl=0) 1
  ...
> .Internal(inspect(values))
@1414d0000 19 VECSXP g1c7 [MARK,REF(65535)] (len=1000000, tl=0)
  @13ef6a238 14 REALSXP g1c1 [MARK,REF(65535)] (len=1, tl=0) 1
  @13ef6a238 14 REALSXP g1c1 [MARK,REF(65535)] (len=1, tl=0) 1
  @13ef6a238 14 REALSXP g1c1 [MARK,REF(65535)] (len=1, tl=0) 1
  @13ef6a238 14 REALSXP g1c1 [MARK,REF(65535)] (len=1, tl=0) 1
  @13ef6a238 14 REALSXP g1c1 [MARK,REF(65535)] (len=1, tl=0) 1
  ...

[lionel-]

We're returning to C here, not to R, so I don't think we can compare these cases.

If ... = !!!x and x is a list, we likely just return it. But we still don't own it, I guess I wanted to return a fresh object to the caller for safety.

It's probably fine to let the caller decide whether to clone. Just need documented behaviour.

[lionel-]

Maybe we can just bump the refcount?

[lionel-]

From zed call: There's no way to change the rlang impl while making sure only updated vctrs uses it (i.e. users may end up with new rlang and old vctrs even if we sync the releases). So we're going to export a new C callable that bumps the refcount but doesn't clone, and keep the current C callable as is.