Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add caching to run command #881

Closed
12 tasks
tiehuis opened this issue Apr 1, 2018 · 2 comments · Fixed by #1494
Closed
12 tasks

Add caching to run command #881

tiehuis opened this issue Apr 1, 2018 · 2 comments · Fixed by #1494
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase.
Milestone

Comments

@tiehuis
Copy link
Member

tiehuis commented Apr 1, 2018

The current zig run command is fairly simplistic right now. It does not perform any caching and will recompile source every invocation. Since this is a large, fairly complicated and only impacts usability/performance of the command, this should only be implemented for the self-hosted compiler.

We need to consider the following ways in which the cache may be invalidated:

  • whether the root source file changed (in name or contents)
  • the args to the compiler
  • whether any of the files that are depended on via @import changed
  • whether any of the files that are depended on via @embedFile changed
  • whether zig binary changed
  • whether any of the dynamic libraries that zig binary depends on changed, and their dynamic library dependencies, recursively
  • whether any of the above things changed, but for the builtin.o target
  • whether any of the above things changed, but for the compiler_rt.o target
  • if the binary does C importing, we have to check if any of the .h files that libclang looked at changed. This may require careful integration with libclang.
  • whether any of the --object or --library files changed, and recursively for dynamic libraries.
  • other command line args that reference files, such as linker scripts

Other possible improvements:

  • handle SIGABRT in the spawned process to get the correct status code returned by the calling zig run process.
@tiehuis tiehuis added the enhancement Solving this issue will likely involve adding new logic or components to the codebase. label Apr 1, 2018
@bnoordhuis
Copy link
Contributor

Is the idea to use file timestamps as a cheap first-pass filter before hashing?

If yes, then a cache entry should be invalidated not only when its timestamp is older but also when it's newer than its on-disk counterpart(s); it probably means the system clock jumped back in time.

@andrewrk
Copy link
Member

andrewrk commented Apr 1, 2018

Instead if mtime I'd like to hash the file contents. We can have a cache of hashed file contents; and we can use mtime equality to determine if the file contents hash needs to be recomputed.

@andrewrk andrewrk added this to the 0.4.0 milestone Apr 4, 2018
@andrewrk andrewrk mentioned this issue Sep 11, 2018
11 tasks
@andrewrk andrewrk modified the milestones: 0.4.0, 0.3.0 Sep 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants