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

PCH timestamp is not restored / ignored #549

Closed
cristianadam opened this issue Feb 26, 2020 · 5 comments
Closed

PCH timestamp is not restored / ignored #549

cristianadam opened this issue Feb 26, 2020 · 5 comments
Labels
bug Does not work as intended/documented closed: won't fix/implement/merge Will not fix/implement/merge

Comments

@cristianadam
Copy link
Contributor

How to reproduce

While working on CMake's MR #4400 PCH: Copy the timestamp from an absolute header file I've noticed that ccache is not restoring the timestamp of the cached PCH file.

I tested with #MR4400 and gcc and a Hello World application:

#include "pch.hpp"

int main()
{
  std::cout << "Hello world\n";
} 

pch.hpp

#pragma once
#include <iostream> 

CMakeLists.txt

cmake_minimum_required(VERSION 3.16)

project(main)

add_executable(main main.cpp)

target_precompile_headers(main PRIVATE pch.hpp)

With the following ccache results:

cache hit (direct)                     1
cache hit (preprocessed)               1
cache miss                             0
cache hit rate                    100.00 %
cleanups performed                     0
files in cache                         3

Actual behavior

The preprocessed file is from ccache not restoring the timestamp of the cmake_pch.gch file:

Include file C:/Projects/HelloWorld/build/CMakeFiles/main.dir/cmake_pch.hxx.gch too new

Expected behavior

ccache should have restored the PCH file timestamp, and have a direct hit for the source file.

Environment

I used my own master (wrong 3.7.7) ccache build.

MinGW GCC.

@cristianadam cristianadam added the bug Does not work as intended/documented label Feb 26, 2020
@jrosdahl
Copy link
Member

jrosdahl commented Mar 1, 2020

ccache should have restored the PCH file timestamp

I'm not so sure about that since that isn't how a compiler behaves: too my knowledge a compiler always creates files with a fresh timestamp. If ccache would remember the timestamp of cmake_pch.hxx.gch and restore it on a cache hit then the timestamp of an updated (but unchanged) cmake_pch.hxx could be newer than that of cmake_pch.hxx.gch, which can violate assumptions and leading to rebuilds. Does this make sense or am I missing something?

There is a sloppiness value called include_file_mtime that you can set to opt out of the "too new include file" check. Would that fix your issue? Or do you see any other solution?

@jrosdahl jrosdahl added the awaiting feedback Waiting for response from issue/PR author label Mar 6, 2020
@cristianadam
Copy link
Contributor Author

Adding include_file_mtime as sloppiness resulted in:

cache hit (direct)                     0
cache hit (preprocessed)               0
cache miss                             2
cache hit rate                      0.00 %

The log file contained:

Include file C:/Projects/HelloWorld/build/CMakeFiles/main.dir/cmake_pch.hxx ctime too new

The log file is here.

Adding include_file_ctime to slopiness resulted also in 0.00% hit rate.

Actually I would like to have ccache ignore the timestap of the gch file, which it already found in the cache.

Otherwise storing a gch file, which afterwards gets re-generated is just a waste of space.

@no-response no-response bot removed the awaiting feedback Waiting for response from issue/PR author label Mar 10, 2020
@cristianadam cristianadam changed the title PCH timestamp is not restored PCH timestamp is not restored / ignored Mar 10, 2020
@jrosdahl
Copy link
Member

Adding include_file_ctime to slopiness resulted also in 0.00% hit rate.

Ah, I forgot about include_file_ctime. Could you provide a cache miss log with include_file_ctime set as well? I would have expected it to work and I don't see how mtime or ctime could have the effect the outcome if both include_file_mtime and include_file_ctime are set.

Actually I would like to have ccache ignore the timestap of the gch file, which it already found in the cache.

Right, I now see the (or at least a) problem: The "include file too new" checks in do_remember_include_file are not supposed to be made for the main (header) file when precompiling a header. I think that ccache may get sidetracked by the use of -c CMakeFiles/main.dir/cmake_pch.hxx.cxx instead of -c CMakeFiles/main.dir/cmake_pch.hxx.

Note that the log talks about too new cmake_pch.hxx, not cmake_pch.hxx.gch or cmake_pch.hxx.cxx, so restoring or ignoring the timestamp of cmake_pch.hxx.gch would not make any difference.

Otherwise storing a gch file, which afterwards gets re-generated is just a waste of space.

The "include file mtime/ctime" checks only affect the direct mode, so I would expect there to be preprocessed cache hits during ≈1 second and then direct mode hits. Thus I suspect that the real problem is something else. To find out what you could try enabling debug mode and comparing the .ccache-input-text files for two compilations of preprocessed headers where you think the second should be a cache hit from the first.

NB: I don't use PCH headers myself so I have to refresh my memory each time a question arises. I may very well be missing something.

Maybe #160 is related to your problem?

@cristianadam
Copy link
Contributor Author

As it turns out I was using a different cmake-dev version, which didn't have the MR #4400 PCH: Copy the timestamp from an absolute header file in.

Now I've retested with include_file_mtime and got:

cache hit (direct)                     2
cache hit (preprocessed)               0
cache miss                             0
cache hit rate                    100.00 %

I would also like to point out that I've backported my CMake changes to 3.7-maint branch. And for testing I've used a 3.7-maint release artifact.

I still have to backport my Visual C++ changes, and I will try to re-release the 3.7.6 and 3.7.7 versions as they should be.

@jrosdahl
Copy link
Member

OK, thanks for the update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Does not work as intended/documented closed: won't fix/implement/merge Will not fix/implement/merge
Projects
None yet
Development

No branches or pull requests

2 participants