simmer() catches parent environment

[olafmersmann]

https://github.com/r-simmer/simmer/blob/f6f58bfe9b1e7a11f8bb87e776fc9390043c0ef2/R/simmer-methods.R#L80-L82

When simmer() creates the initial simulation environment using list2env(), it captures the parent frame and all frames above that. As far as I can tell, the parent environment is never referenced. Would it make sense to add parent=emptyenv()?

https://github.com/r-simmer/simmer/blob/f6f58bfe9b1e7a11f8bb87e776fc9390043c0ef2/R/wrap-methods.R#L55-L71

The same thing happens in wrap(). Again, adding parent=emptyenv(), seems to do no harm.

[Enchufa2]

Well, the good thing of capturing the parent environment is that, if you save a wrapped object:

library(simmer)
env <- wrap(simmer())
save(env, file="test.RData")

and then you load it in a fresh R session, the package is loaded automatically, because the simmer namespace is one of the parents:

load("test.RData")
env
#> simmer environment: anonymous | now: 0 | next: 
#> { Monitor:  }

Instead, if we use an empty environment as the parent, you see this unless you explicitly load simmer:

load("test.RData")
env
#> <environment: 0x55ffac460fb0>
#> attr(,"class")
#> [1] "wrap"

which is not ideal. This is certainly a minor benefit, but, on the other hand, I cannot see any drawback. How did you arrive to this? Did you experience any issue due to this behaviour?

[olafmersmann]

Sorry for dropping the ball on this and I see your point about attaching the namespace when loading the environment. Would you at least consider releasing the original simulation environment (.env) in warp()? Otherwise (from my understanding of the R internals) the sim_obj ExternalPtr in the original environment is never released.

[Enchufa2]

wrap is meant to be used in parallel simulations. Even if sim_obj is catched, the memory was released and it's just an invalid pointer. But yes, I suppose that it's reasonable to set something like parent=asNamespace("simmer") in such a case.

Sorry to ask again, but I'm curious whether you found issues due to this.

[olafmersmann]

Yes and no. We have a large(ish) simulation which leaks memory somewhere. One hypothesis was that we were never freeing the state in the C++ simulator when we do many repeated simulations (we are not using reset(), instead, for historical reasons, we construct a new simulation environment for each run) and this is one of the things that reduced the memory usage but did not fix it.

I'm still investigating this and will post a minimal example if/when I can pinpoint the problem.

[Enchufa2]

Thanks, I'll implement this in wrap. Please, keep me posted about new findings. I ask because we have a workaround in place due to a bug in magrittr that leaks references and thus keeps memory from being gc'ed. It has been reported for some time now, and recently they explicitly refused to solve it. So I wouldn't discard that it could be some other problem with magrittr. I hope that this is solved in the new native pipe that is planned for the next version of R.

[Enchufa2]

Closing this per the commit above. Please, let me know if you progress in your search for the leak, and don't hesitate to open a new issue in such a case.