Skip to content

Conversation

@xymus
Copy link
Contributor

@xymus xymus commented Jul 20, 2020

Avoid mmaping swiftmodule files to hopefully fix issues seen when building Swift projects in parallel on NFS. This change only affects loading ModuleFile, it doesn't affect scanning swiftmodule for dependencies which are still handled as non-volatile.

Let's try always avoid mmap at first to confirm if this was the issue. However, this will likely affect memory usage so we may need a way to restrict this further if it causes issues on memory-constrained systems.

@xymus
Copy link
Contributor Author

xymus commented Jul 20, 2020

@swift-ci Please smoke test

Avoid mmaping swiftmodule files to hopefully fix issues seen when
building many Swift projects in parallel on NFS. This only affects
loading ModuleFile, it doesn't affect scanning swiftmodule for
dependecies which are still handled as non-volatile.

rdar://63755989
@xymus xymus force-pushed the swiftmodule-files-are-volatile branch from 15a797b to a4becda Compare July 21, 2020 17:35
@xymus
Copy link
Contributor Author

xymus commented Jul 21, 2020

@swift-ci Please smoke test

@xymus xymus requested review from beccadax and nkcsgexi and removed request for beccadax July 21, 2020 17:40
@nkcsgexi
Copy link
Contributor

Could you elaborate more about what kind of issues in parallel building could potentially be resolved by avoiding mmaping swiftmodule file?

@xymus
Copy link
Contributor Author

xymus commented Jul 21, 2020

Right, I believe the issue here is that swiftmodule files are being changed by a different process while they are mmaped. The precise behavior of mmap when the underlying file changes is unspecified AFAICT. This setting in LLVM mentions specifically issues on NFS and it could be an important factor here as we don't see this issue on engineers' desks.

And in practice, there may be three scenarios where the swiftmodule files can change while it's being read:

  • For a framework without installapi, a client could be using the baseline swiftmodule while the framework replaces it on disk if there is a missing dependency edge.
  • For a framework with installapi, if the framework reinstalls a swiftmodule in the install phase while a client is reading the one from the installapi phase.
  • When lock files are unavailable because of a read-only file system and many processes rebuild the same module interface at once. I would have expected this one to be safe but we often see this error after rebuilds and failed locks.

@xymus
Copy link
Contributor Author

xymus commented Jul 21, 2020

This would explain failures that show up as deserialization errors without an explanation which indicates that it's a memory consistency issue: (An expected deserialization error would have an explanation between those two lines.)

*** DESERIALIZATION FAILURE (please include this section in any bug report) ***
(see "While..." info below)

And low-level failures in LLVM bitstream code from reading unexpected values, and segfaults when using values from the swiftmodule files for index access.

@nkcsgexi
Copy link
Contributor

OK. Though I'm not entirely convinced using mmap really caused these deserialization errors, it's worth experimenting by landing this change. Could you add the rationals as comments and file a radar to revert this change if similar deserialization errors still happen?

@xymus
Copy link
Contributor Author

xymus commented Jul 21, 2020

@swift-ci Please smoke test

@xymus
Copy link
Contributor Author

xymus commented Jul 21, 2020

@nkcsgexi Done!

Copy link
Contributor

@nkcsgexi nkcsgexi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants