perf(sqlx-cli): avoid rewriting the entire .sqlx cache on prepare#4237
perf(sqlx-cli): avoid rewriting the entire .sqlx cache on prepare#4237fhsgoncalves wants to merge 1 commit into
Conversation
…nd add or update when needed
fhsgoncalves
force-pushed
the
perf/prepare-prune
branch
from
ba5071a to
eafb17c
Compare
jplatte
left a comment
jplatte
left a comment
There was a problem hiding this comment.
Why does the commit message say perf? This doesn't actually help performance in any way, right?
|
|
||
| let prepare_dir = ctx.prepare_dir()?; | ||
| run_prepare_step(ctx, &prepare_dir)?; | ||
| let cache_dir = ctx.metadata.target_directory().join("sqlx-prepare"); |
There was a problem hiding this comment.
I think this should probably be a subdirectory of .sqlx instead, to avoid any cross-filesystem copying in case target is a symlink to a different fs. Maybe .sqlx/staging? You could put a .gitignore in .sqlx automatically to not have it tracked by git (though maybe this would be annoying for people on other version control systems?).
There was a problem hiding this comment.
Hm, do we already have this kind of temporary folder somewhere here in sqlx?
Using .sqlx looks weird for me, since it is a folder to be versioned, and may impact people not using git indeed, or already having an own custom .gitignore inside .sqlx/ folder.
About using the target directory: why that would be an issue with cross-filesystem copying? It should work, right?
There was a problem hiding this comment.
Cross-fs copies are slower and more error-prone because they're not atomic. I could see things breaking if in parallel with sqlx prepare, rust-analyzer executes one of the macros or something.
There was a problem hiding this comment.
Ok! Which is the best option? I can change the code, no problem.
I still prefer to save on target/ since it is a temporary folder on any cargo project, but the two options have pros and cons.
There was a problem hiding this comment.
We used to use target/ but then had to change: #2663
I've been wondering, is there a potential benefit to bypassing the filesystem entirely here? E.g. we open a TCP socket on an ephemeral port and the macros connect to it and send their data over that, then we collect files in-memory and only then decide whether to write them out.
I already had something similar in mind after considering the discussion in #1598: the macros wouldn't even need to connect to the database themselves, but would instead ask sqlx-cli to do it for them over this link. This would allow third-party drivers to integrate with the macros without having to provide a whole new facade (you could just cargo install the extra driver binaries). And it would potentially be a huge speed-boost because sqlx-cli would be automatically compiled in release mode and could keep database connections open even across compiler invocations.
There was a problem hiding this comment.
I'm a bit skeptical about doing it all in-memory. Rust development is already notorious for using loads of RAM.
Re. the indirect DB connection, sounds like a useful addition but unrelated to this PR, right?
There was a problem hiding this comment.
The files would only need to be held in-memory until we decided whether to write them out or discard them. Then we'd just store content hashes to check for duplicate writes.
Re. the indirect DB connection, sounds like a useful addition but unrelated to this PR, right?
Of course, it's just another use-case for having the macros talk directly to sqlx-cli.
It is because the dev-loop when using |
|
I'm not actually a maintainer, so I can't make any definitive statements about commit message style. Was just asking out of curiosity. |
|
Hey maintainers! Did you have a chance to look this PR? 😄 |
|
ping @abonander |
3 participants
Does your PR solve an issue?
N/A (no issue filed).
Is this a breaking change?
No.
Summary
query-*.jsonfiles in.sqlxon everycargo sqlx prepare.sqlxMotivation: I'm working on a large monorepo, with ~1500 sqlx cached query files, and every time I run
cargo sqlx prepare --workspace, it deletes all the cached files, the IDE/lsp shows lots of compilation errors, and I need to wait several seconds (sometimes minutes) to have all the files back. This PR fixes that