gtristan commented on PR #1991:
URL: https://github.com/apache/buildstream/pull/1991#issuecomment-2995324658

   > In an ideal world, I'd prefer that `Element.stage_directory()` take a 
`Directory` rather than a `Sandbox`. Then for this MR, I'd want a way for a 
plugin to create a cas-based `Directory` instance. But we're stuck with 
`Element.stage_directory()` taking a `Sandbox` for now.
   
   Right, I think you mean `Element.stage_artifact()`, and yeah it *might* be 
cool if this was `Directory` based.
   
   On the surface, I feel mostly concerned with the overhead of *staging the 
artifact*. This activity, at least on the surface, looks like an activity which 
involves I/O, when all we are actually looking for is *what CAS digest would 
the root `Directory` have, if this set of artifacts were to be staged*.
   
   Looking at the operation, I see that we're essentially using 
`CasBasedDirectory._import_files_internal(Directory, ...)`. For a 
`CasBasedDirectory` being imported - this essentially lands up in 
`CasBasedDirectory.__partial_import_cas_into_cas()`.
   
   While the code around here is a bit tricky to follow, I think I'm not really 
seeing any I/O, instead what generally happens during a build is:
   * `Sanbox` creates it's vdir
   * Elements stage artifacts into that vdir
   * This process only plays with data structures in the vdir 
`CasBasedDirectory` object hierarchy
   * `SandboxREAPI._run()` then serializes the in memory `CasBasedDirectory` 
into protobufs and delegates this to `SandboxREAPI._execute_action`, which can 
be handled by either `SandboxBuildboxRun` or `SandboxRemote`
   
   So it looks like the I/O of managing the cache only ever happens immediately 
before a build activity or after when we cache the artifact.
   
   In summary, I think I've looked at this enough to feel fairly confident that 
this activity is not really I/O intensive.
   
   > 
   > I'm happy to do it differently if you can think of a better way of doing 
it.
   
   I think I'm pretty happy with this approach using a dummy sandbox for this, 
after having looked deeper into the implementation details :)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to