search

openllb / hlb

A developer-first language to build and test any software efficiently
https://openllb.github.io/hlb/
Apache License 2.0
108 stars 12 forks source link

remove async solves for remote-directories #325

Closed coryb closed 2 years ago

coryb commented 2 years ago

The ast.Directory Close is never called, so that ends up holding the Solves running for the entire build. See: https://github.com/openllb/hlb/pull/323#issuecomment-1190974238

For now removing Close from the interface until we can figure out how to more narrowly scope the remote directory usage. This also moves the file lookups to be synchronous on use (so still lazy if never used). This could have a large impact if we were to access many files from the remote directory, but typically I think we use this to just import the module.hlb file for a remote import.

Future optimizations might be to ref-count the solve so we can reuse the single solve for the life time of all the files returned from Directory.Open.

coryb commented 2 years ago

@hinshun I think this is a better approach than removing async altogether, we still get most of the benefits of your async updates, but just the remote directories are solved sync when needed, which I think we can optimize further if it turns out to be a bottleneck.

I was able to deploy this internally and it seems to solve all the issues we have been having 🤞

hinshun commented 2 years ago

This looks like a reasonable fix. Sorry for the trouble caused!