This is an archive of the discontinued LLVM Phabricator instance.

Module: retry building modules that were just compiled by the same instance and are are out of date
AbandonedPublic

Authored by manmanren on Jul 21 2016, 11:15 AM.

Details

Summary

Even though this instance just built module "A", it is likely that "A" imports another module "B" and B.pcm becomes out of date when we try to load module "A", because another instance overwrites "B.pcm" due to changes in warning options. This patch tries to fix the error: "Module file ‘<some-file path>.pcm' is out of date and needs to be rebuilt" by simply trying again.

This seems to be the last case where we call ReadAST without the ability to handle out-of-date.

If there are other ways to fix the issue, please let me know.
Also I don't quite know how to add a testing case when two compiling instances are involved and running in parallel.

rdar://problem/26676111

Diff Detail

Event Timeline

manmanren updated this revision to Diff 64925.Jul 21 2016, 11:15 AM
manmanren retitled this revision from to Module: retry building modules that were just compiled by the same instance and are are out of date.
manmanren updated this object.
manmanren added reviewers: benlangmuir, rsmith.
manmanren added a subscriber: cfe-commits.
benlangmuir edited edge metadata.Jul 21 2016, 11:36 AM

B.pcm becomes out of date when we try to load module "A", because another instance overwrites "B.pcm"

How can this happen? We intentionally keep the file descriptors of modules open. If you just built A, then you will have B.pcm still open. When you read A, you will use the same B that you used when you built A even if the file on disk has been replaced (and we use rename to replace the file, so the existing one is never modified).

B.pcm becomes out of date when we try to load module "A", because another instance overwrites "B.pcm"

How can this happen? We intentionally keep the file descriptors of modules open. If you just built A, then you will have B.pcm still open. When you read A, you will use the same B that you used when you built A even if the file on disk has been replaced (and we use rename to replace the file, so the existing one is never modified).

Can you point me to the source codes where we use rename to replace the file? I am curious on how this all works out.

What I described is a scenario I thought possible that can cause "out-of-date" error:
module "B" is out of date and needs to be rebuilt
note: imported by module "A"

The only invocation of ReadAST that reads a module file and can't handle out-of-date modules, is the path where we just built module "A" and tries to load module "A" (here ModuleLoadCapabilities will be ARR_Missing).

I am still working with the project owner to collect more debugging messages.

If this is not a possible scenario, do you have any suggestion on what can cause this error?

Cheers,
Manman

Can you point me to the source codes where we use rename to replace the file? I am curious on how this all works out.

This is the same as clang's handling of other output files. See {{CompilerInstance::createOutputFile}} and {{clearOutputFiles}}. When we are performing the GenerateModuleAction, the .pcm file will be the output file.

What I described is a scenario I thought possible that can cause "out-of-date" error:
module "B" is out of date and needs to be rebuilt
note: imported by module "A"
The only invocation of ReadAST that reads a module file and can't handle out-of-date modules, is the path where we just built module "A" and tries to load module "A" (here ModuleLoadCapabilities will be ARR_Missing).

Right, here and in PCH loading (I assume your case doesn't involve a PCH). I'm concerned about adding this loop, because it removes the guarantee of progress. We should never need to build a module more than once in the same compilation. If we have a bug that's causing us to not be able to load a module that we just compiled, I think we need to fix the underlying problem, not just retry the build.

Can you point me to the source codes where we use rename to replace the file? I am curious on how this all works out.

This is the same as clang's handling of other output files. See {{CompilerInstance::createOutputFile}} and {{clearOutputFiles}}. When we are performing the GenerateModuleAction, the .pcm file will be the output file.

Thanks for the info!

What I described is a scenario I thought possible that can cause "out-of-date" error:
module "B" is out of date and needs to be rebuilt
note: imported by module "A"
The only invocation of ReadAST that reads a module file and can't handle out-of-date modules, is the path where we just built module "A" and tries to load module "A" (here ModuleLoadCapabilities will be ARR_Missing).

Right, here and in PCH loading (I assume your case doesn't involve a PCH). I'm concerned about adding this loop, because it removes the guarantee of progress. We should never need to build a module more than once in the same compilation. If we have a bug that's causing us to not be able to load a module that we just compiled, I think we need to fix the underlying problem, not just retry the build.

Yes, I agree. We should try to figure out the root cause. I gave the project owner a root with debugging messages, but with this root, the error disappeared :(

If you have suggestions on debugging this, let me know.

Thanks,
Manman

manmanren abandoned this revision.Jul 27 2016, 3:31 PM

Abandon this change for now.