Re: [ccache] PATCH: Look at include files' mtimes
FWIW I've been using ccache with mtime checking for the past few weeks and I haven't noticed any problems. That is a pretty low bar, I admit, but it's something. :) On Sun, Mar 3, 2013 at 3:54 PM, Joel Rosdahl j...@rosdahl.net wrote: Hi Justin, I've resurrected these patches to look at files' mtimes and ctimes. [...] I just found out that I forgot to have a look at your patches. Sorry about the delay. I seem fine, so I've applied them. I did need to fix the unit tests since they failed, though. Please have a look and see if it looks all right. Thanks, -- Joel On 25 December 2012 08:18, Justin Lebar justin.le...@gmail.com wrote: Hi, all. I've resurrected these patches to look at files' mtimes and ctimes. Hopefully the three patches here (with their commit messages) don't need further explanation. Note that the second patch here increases safety for everyone, not just those who choose to have mtime matching on. These patches seem to be working, but I'm not seeing a significant speedup on my Mac. I think that may be a separate issue, as this machine isn't particularly good at I/O. I don't have access to my Linux box for a while, so I'd certainly appreciate if someone could verify whether there's a speedup here. I'd also appreciate if some of you could test this patch by turning on CCACHE_SLOPPINESS=file_stat_matches and letting me know if you have any problems. Happy holidays. -Justin On Sun, May 20, 2012 at 4:49 PM, Justin Lebar justin.le...@gmail.com wrote: This patch lets ccache examine an include file's mtime and size in lieu of hashing it, during direct mode. If the mtime and size don't match, we fall back to hashing. The net result is roughly a factor-of-two speedup in ccache hits (*), on my machine. I'm not sure if this is a desirable feature, because obviously mtimes can be tampered with. I didn't provide a way to disable the feature in this patch because, presuming we wanted to take this patch, I'm not sure if we'd want mtime-snooping enabled by default. Since most projects already rely on accurate mtimes in their build systems, turning this on by default doesn't seem particularly outrageous to me. Please let me know what you think about this. Regards, -Justin (*) Experimental procedure: In a Firefox debug objdir (CCACHE_HARDLINK, Linux-64, Ubuntu 12.04, 4 CPU cores), let * Let |nop| be the average real time from a few runs of $ time make -C dom -sj16 when there's nothing to do. * Let |orig| be the average real time from a few runs of $ find dom -name '*.o' time make -C dom -sj16 with ccache master (701f13192ee) (discarding the first run, of course). * Let |mtime| be the real time from the same command as |orig|, but with patched ccache. Speedup is (orig - nop) / (mtime - nop). On my machine, nop = 3.71, orig = 4.88, mtime = 4.31. Yes, our nop build times are atrocious. ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
Re: [ccache] PATCH: Look at include files' mtimes
Hi Justin, I've resurrected these patches to look at files' mtimes and ctimes. [...] I just found out that I forgot to have a look at your patches. Sorry about the delay. I seem fine, so I've applied them. I did need to fix the unit tests since they failed, though. Please have a look and see if it looks all right. Thanks, -- Joel On 25 December 2012 08:18, Justin Lebar justin.le...@gmail.com wrote: Hi, all. I've resurrected these patches to look at files' mtimes and ctimes. Hopefully the three patches here (with their commit messages) don't need further explanation. Note that the second patch here increases safety for everyone, not just those who choose to have mtime matching on. These patches seem to be working, but I'm not seeing a significant speedup on my Mac. I think that may be a separate issue, as this machine isn't particularly good at I/O. I don't have access to my Linux box for a while, so I'd certainly appreciate if someone could verify whether there's a speedup here. I'd also appreciate if some of you could test this patch by turning on CCACHE_SLOPPINESS=file_stat_matches and letting me know if you have any problems. Happy holidays. -Justin On Sun, May 20, 2012 at 4:49 PM, Justin Lebar justin.le...@gmail.com wrote: This patch lets ccache examine an include file's mtime and size in lieu of hashing it, during direct mode. If the mtime and size don't match, we fall back to hashing. The net result is roughly a factor-of-two speedup in ccache hits (*), on my machine. I'm not sure if this is a desirable feature, because obviously mtimes can be tampered with. I didn't provide a way to disable the feature in this patch because, presuming we wanted to take this patch, I'm not sure if we'd want mtime-snooping enabled by default. Since most projects already rely on accurate mtimes in their build systems, turning this on by default doesn't seem particularly outrageous to me. Please let me know what you think about this. Regards, -Justin (*) Experimental procedure: In a Firefox debug objdir (CCACHE_HARDLINK, Linux-64, Ubuntu 12.04, 4 CPU cores), let * Let |nop| be the average real time from a few runs of $ time make -C dom -sj16 when there's nothing to do. * Let |orig| be the average real time from a few runs of $ find dom -name '*.o' time make -C dom -sj16 with ccache master (701f13192ee) (discarding the first run, of course). * Let |mtime| be the real time from the same command as |orig|, but with patched ccache. Speedup is (orig - nop) / (mtime - nop). On my machine, nop = 3.71, orig = 4.88, mtime = 4.31. Yes, our nop build times are atrocious. ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
Re: [ccache] PATCH: Look at include files' mtimes
Hi, all.
I've resurrected these patches to look at files' mtimes and ctimes.
Hopefully the three patches here (with their commit messages) don't
need further explanation. Note that the second patch here increases
safety for everyone, not just those who choose to have mtime matching
on.
These patches seem to be working, but I'm not seeing a significant
speedup on my Mac. I think that may be a separate issue, as this
machine isn't particularly good at I/O. I don't have access to my
Linux box for a while, so I'd certainly appreciate if someone could
verify whether there's a speedup here.
I'd also appreciate if some of you could test this patch by turning on
CCACHE_SLOPPINESS=file_stat_matches and letting me know if you have
any problems.
Happy holidays.
-Justin
On Sun, May 20, 2012 at 4:49 PM, Justin Lebar justin.le...@gmail.com wrote:
This patch lets ccache examine an include file's mtime and size in
lieu of hashing it, during direct mode. If the mtime and size don't
match, we fall back to hashing.
The net result is roughly a factor-of-two speedup in ccache hits (*),
on my machine.
I'm not sure if this is a desirable feature, because obviously mtimes
can be tampered with.
I didn't provide a way to disable the feature in this patch because,
presuming we wanted to take this patch, I'm not sure if we'd want
mtime-snooping enabled by default. Since most projects already rely
on accurate mtimes in their build systems, turning this on by default
doesn't seem particularly outrageous to me.
Please let me know what you think about this.
Regards,
-Justin
(*) Experimental procedure: In a Firefox debug objdir
(CCACHE_HARDLINK, Linux-64, Ubuntu 12.04, 4 CPU cores), let
* Let |nop| be the average real time from a few runs of
$ time make -C dom -sj16
when there's nothing to do.
* Let |orig| be the average real time from a few runs of
$ find dom -name '*.o' time make -C dom -sj16
with ccache master (701f13192ee) (discarding the first run, of course).
* Let |mtime| be the real time from the same command as |orig|, but
with patched ccache.
Speedup is (orig - nop) / (mtime - nop). On my machine, nop = 3.71,
orig = 4.88, mtime = 4.31. Yes, our nop build times are atrocious.
From ecac7e50e02d4a8319caff9faf4dbb9527a182e1 Mon Sep 17 00:00:00 2001
From: Justin Lebar justin.le...@gmail.com
Date: Mon, 24 Dec 2012 16:16:46 -0500
Subject: [PATCH 1/3] Extern time_of_compilation.
---
ccache.c | 2 +-
ccache.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/ccache.c b/ccache.c
index eceedff..b77f999 100644
--- a/ccache.c
+++ b/ccache.c
@@ -138,7 +138,7 @@ static char *manifest_path;
* Time of compilation. Used to see if include files have changed after
* compilation.
*/
-static time_t time_of_compilation;
+time_t time_of_compilation;
/*
* Files included by the preprocessor and their hashes/sizes. Key: file path.
diff --git a/ccache.h b/ccache.h
index 18a2b9e..5bcbf71 100644
--- a/ccache.h
+++ b/ccache.h
@@ -213,6 +213,7 @@ void lockfile_release(const char *path);
/* - */
/* ccache.c */
+extern time_t time_of_compilation;
bool cc_process_args(struct args *args, struct args **preprocessor_args,
struct args **compiler_args);
void cc_reset(void);
--
1.8.0
From 6c2ffcab24538b00522dae63c74ff3d4ec960467 Mon Sep 17 00:00:00 2001
From: Justin Lebar justin.le...@gmail.com
Date: Mon, 24 Dec 2012 23:09:14 -0500
Subject: [PATCH 2/3] Check that included files' ctimes aren't too new.
ccache currently checks that a file's mtime isn't too new, unless
CCACHE_SLOPPINESS includes include_file_mtime.
This patch adds a similar check that a file's ctime isn't too new. We
skip this check if CCACHE_SLOPPINESS includes include_file_ctime.
---
MANUAL.txt | 3 +++
ccache.c | 6 ++
ccache.h | 5 +++--
conf.c | 5 +
test.sh | 51 ---
test/test_conf.c | 6 --
6 files changed, 49 insertions(+), 27 deletions(-)
diff --git a/MANUAL.txt b/MANUAL.txt
index 3a4afde..e7411b4 100644
--- a/MANUAL.txt
+++ b/MANUAL.txt
@@ -428,6 +428,9 @@ WRAPPERS.
*include_file_mtime*::
By default, ccache will not cache a file if it includes a header whose
mtime is too new. This option disables that check.
+*include_file_ctime*::
+ccache also will not cache a file if it includes a header whose ctime is
+too new. This option disables that check.
*time_macros*::
Ignore *\_\_DATE\__* and *\_\_TIME__* being present in the source code.
--
diff --git a/ccache.c b/ccache.c
index b77f999..22b056c 100644
--- a/ccache.c
+++ b/ccache.c
@@ -373,6 +373,12 @@ remember_include_file(char *path, struct mdfour *cpp_hash)
goto failure;
}
+ if (!(conf-sloppiness SLOPPY_INCLUDE_FILE_CTIME)
+st.st_ctime = time_of_compilation) {
+
Re: [ccache] PATCH: Look at include files' mtimes
I can't say I'm particularly interested in supporting two manifest versions simultaneously I agree that supporting only the latest should be enough. Coming to think of it, perhaps it would be a good idea to include the manifest version in the manifest hash so that ccache versions using different manifest versions won't even try to read each others' manifest files? (Currently, ccache overwrites incompatible manifest files with its own version.) Allowing multiple versions to co-reside and not trash each other's result would be useful. Presently I need redefine CCACHE_DIR for each version to avoid overlap, which is annoying. (On a large multi-os cluster it often isn't possible to roll out a new version simultaneously.) Although... if the stats format changes overlap must still be avoided. Including the version number in the stats filename would solve that too. ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
Re: [ccache] PATCH: Look at include files' mtimes
Allowing multiple versions to co-reside and not trash each other's result would be useful. =A0Presently I need redefine CCACHE_DIR for each version to avoid overlap, which is annoying. [...] From the top of my head, the issues I'm aware of regarding different ccache versions accessing the same cache directory are: * 3.1 throws away statistics counters from future ccache versions, which is annoying but shouldn't be critical. * 3.1 and =3.1 have different methods of locking and updating the stats files. If you're unlucky, the stats files (including the cache size limits) could be garbled. Do you know of more issues than the above? I don't know of specific recent issues. ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
Re: [ccache] PATCH: Look at include files' mtimes
On 22 May 2012 04:00, Justin Lebar justin.le...@gmail.com wrote: I can't say I'm particularly interested in supporting two manifest versions simultaneously I agree that supporting only the latest should be enough. Coming to think of it, perhaps it would be a good idea to include the manifest version in the manifest hash so that ccache versions using different manifest versions won't even try to read each others' manifest files? (Currently, ccache overwrites incompatible manifest files with its own version.) -- Joel ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
Re: [ccache] PATCH: Look at include files' mtimes
On 22 May 2012 12:00, Justin Lebar justin.le...@gmail.com wrote: I've been burned by mtime only checking before as (excluding some recent file systems) mtime has a resolution only down to one second. I tried to address this in the patch, although come to think of it, I did it wrong. The trick is only to *cache* mtimes that are at least one second older than now. Then the resolution of the clock isn't a problem. Better to do 2 seconds, since FAT (and maybe some other Windows related setups) has only a 2-second resolution. The other thing you can do is, on Unix, use the latest of ctime and mtime, which should catch cases where the mtime gets reset. But if the system clock is set back (e.g. by NTP), we're in trouble. That's true, but it's a pretty bad idea to make large time steps on a build machine... And hardlinks are often created without bumping the inode's mtime, which is also problematic. (It's problematic for make, too.) ctime will catch that. -- Martin ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
Re: [ccache] PATCH: Look at include files' mtimes
Better to do 2 seconds, since FAT (and maybe some other Windows related setups) has only a 2-second resolution. The other thing you can do is, on Unix, use the latest of ctime and mtime, which should catch cases where the mtime gets reset. Thanks for the tips! I'm happy to update the patch, but I'd first want to hear Joel's thoughts. -Justin ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
