[PATCH] Verify source files for a module only once during the build

[Argyrios Kyrtzidis]

On Feb 7, 2014, at 12:47 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:

> On Fri, Feb 7, 2014 at 6:31 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com> wrote:
>> +def fmodules_skip_verify_if_validated_after : Joined<["-"],
>> "fmodules-skip-verify-if-validated-after=">,
>> 
>> Should we use a more general name, like “-fbuild-session-timestamp” or
>> something, this seems like it could have other uses in the future.
> 
> I like the current name because clang does what the option name says.
> If build session timestamp could be used for more features, I'd prefer
> to have two options then:
> 
> -fbuild-session-timestamp=... -fmodules-validate-once-per-build-session

Ok, fair enough.

> 
>> +static void updateModuleTimestamp(ModuleFile &MF) {
>> +  // Overwrite the timestamp file contents so that file's mtime changes.
>> +  std::string TimestampFilename = MF.getTimestampFilename();
>> +  std::string ErrorInfo;
>> +  llvm::raw_fd_ostream OS(TimestampFilename.c_str(), ErrorInfo);
>> +  if (!ErrorInfo.empty())
>> +    return;
>> +  OS << "Timestamp file\n";
>> +}
>> 
>> +    New->InputFilesValidationTimestamp = 0;
>> +    if (New->Kind == MK_Module) {
>> +      std::string TimestampFilename = New->getTimestampFilename();
>> +      llvm::sys::fs::file_status Status;
>> +      // A cached stat value would be fine as well.
>> +      if (!FileMgr.getNoncachedStatValue(TimestampFilename, Status)) {
>> +        llvm::sys::TimeValue MTime = Status.getLastModificationTime();
>> +        New->InputFilesValidationTimestamp =
>> +            MTime.toEpochTime() * 1000 + MTime.milliseconds();
>> +      }
>> +    }
>> +
>> 
>> I’m concerned that we are depending on being in-sync with the modification
>> time given by the file system and the builder that provided the original
>> timestamp.
> 
> I wanted to avoid the need to do the atomic-rename dance.

What is you concern with it ?

>  What is the
> potential for mtime confusion that you see?  We could provide a
> function in libclang to get the current timestamp so that clients
> don't have to invent their own, potentially incorrect way to get it.

I really want to reduce complexity here and potential for out-of-sync, because now you have

1) The builder needs to provide an increasing timestamp by getting clock time (or libclang call ?)
2) we will compare that clock time with the file system modification time which can come from any kind of underlying file system

vs

1) The builder needs to provide an increasing timestamp

I much prefer the latter simpler approach.

> 
> Dmitri
> 
> -- 
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140207/3917a227/attachment.html>