Closed busstoptaktik closed 9 months ago
Also, the plain context could handle grids on behalf of the operations: Since we cannot destroy an operation, in any other way than dropping the context itself, there is no ownership problems here
I think the grid-handling in Plain
needs some further explanation (not necessarily in a Rumination - a comment in the code would suffice): To me it looks like we should store either the raw grid-as-bytes, or (probably preferably) the final dyn Grid
as Arc
s in a Hashmap on first request, and then hand out Arc::clone()
s to any Op
requesting it.
If I understand correctly, we just make a new copy, and then needlessly Arc::clone()
it every time a request is made.
Ideally we would want the Arc to self-destruct when the last Op
holding a ref to it is destroyed. But as we as-of A.D. do not have an Context::op_destroy() method, this first happens once the Context
goes out of scope. Nevertheless we might as well keep a list of Arc<Grid>
s and then hand off Arc::clone()
s to any Op
asking for it?
Do you agree with this @Rennzie, or have I gravely misunderstood something?
This is probably me not fully understanding Rust, but I thought what the code was doing is what you describe here:
Nevertheless we might as well keep a list of Arc
s and then hand off Arc::clone()s to any Op asking for it?
Where when the operators gridshift
or deformation
are constructed they requests a grid from the context and get back Arc::new(grid)
(for each available grid). These references are then stored in ParsedParams.grid
which is Vec<Arc<Grid>>
. This means when the operator is used the new
or inv
functions get the vector of grids and go on their merry way.
IIUC Plain
moves the grid pointer to ParsedParams
during operator construction. In geodesy-wasm
it works a little different. Because there isn't way a of loading a grid after the context is constructed (ie no access to the file system or until I build a way of making an http request) it must be loaded when it's created. Here I've done as you've suggested i.e store the grids in a HashMap on the conext and hand out Arc::clones()
when WasmContext.get_grid() is called on operator construction.
Where when the operators gridshift or deformation are constructed they requests a grid from the context and get back
Arc::new(grid)
(for each available grid).
Yes, but this Arc::new(grid)
is provided by the Context
for the sole use of this Op
, so it does not need to be reference counted at all, since every Op
get their own copy.
These references are then stored in ParsedParams.grid which is Vec<Arc
>.
Which is all as expected - but the Arc<Grid>
should just be a light-weight clone (i.e. an Arc
) of an Arc<Grid>
handled by the Context
, such that the same backing heap-store can be used by more than one Op
. Apparently exactly as you have done with the WasmContext
🙂
So for now, I will just carry on and implement the WasmContext
functionality for Plain
- perhaps the two will actually end up being identical, once I learn to handle Wasm as well 🙂
Minimal
andPlain
both need documentation. Perhaps taking off from some documentation of theContext
trait itself.Note that the
Maximal
example in thetests
-folder comprises a fully self contained demonstration of how to implement a user defined context provider.