Skip to content

[ScanDependencies] Get accurate macro dependency #73421

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

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

cachemeifyoucan
Copy link
Contributor

Build an accurate macro dependency for swift caching. Specifically, do not include not used macro plugins into the dependency, which might cause false negatives for cache hits.

This also builds the foundation for future improvement when dependency scanning will determine the macro plugin to load and swift-frontend do not need to redo the work.

rdar://127116512

@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please smoke test

@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please smoke test

@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please smoke test

@xedin xedin removed their request for review May 8, 2024 16:23
@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please smoke test

@cachemeifyoucan
Copy link
Contributor Author

ping

Copy link
Contributor

@benlangmuir benlangmuir left a comment

Choose a reason for hiding this comment

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

Putting timestamps in the cache key seems really unfortunate; won't this miss any time you rebuild a macro plugin even if it hasn't changed (even if it's cached, the timestamp will change)? We had talked about using the binary UUID on mach-o; not sure if elf/coff has similar concepts; or we could hash the binary -- that might miss dynamic dependencies, but I suspect that's rare for macros.

@cachemeifyoucan
Copy link
Contributor Author

The timestamp is a prototype that is not actually used in the actual logic. The idea is to find a better identity function when the plugin loader needs to actually verify the plugin status.

@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please smoke test

@cachemeifyoucan
Copy link
Contributor Author

ping

@benlangmuir
Copy link
Contributor

The timestamp is a prototype that is not actually used in the actual logic.

I'm not sure I understand -- isn't the implementation using timestamp right now? Are you saying this whole change is prototype, or what's the idea?

@cachemeifyoucan
Copy link
Contributor Author

The timestamp is a prototype that is not actually used in the actual logic.

I'm not sure I understand -- isn't the implementation using timestamp right now? Are you saying this whole change is prototype, or what's the idea?

It is a placeholder for future better implementation. The result of the timestamp is actually not used. I can sure remove it.

@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please smoke test

@cachemeifyoucan
Copy link
Contributor Author

Oh, I might need to completely rewrite this after #73725

@cachemeifyoucan cachemeifyoucan force-pushed the eng/PR-127116512 branch 3 times, most recently from bce2345 to d4b8af9 Compare June 27, 2024 13:50
@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

@artemcm artemcm left a comment

Choose a reason for hiding this comment

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

LGTM

Build an accurate macro dependency for swift caching. Specifically, do
not include not used macro plugins into the dependency, which might
cause false negatives for cache hits.

This also builds the foundation for future improvement when dependency
scanning will determine the macro plugin to load and swift-frontend do
not need to redo the work.

rdar://127116512
@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please smoke test

@cachemeifyoucan
Copy link
Contributor Author

@swift-ci please smoke test windows platform

@cachemeifyoucan cachemeifyoucan merged commit 39cb996 into swiftlang:main Jun 27, 2024
2 of 3 checks passed
cachemeifyoucan added a commit to cachemeifyoucan/swift that referenced this pull request Jul 8, 2024
Fix few issues from previous implementation from explicit module build
with macros and accurate macro dependency scanning in
swiftlang#73421.

First, there is a crash when propagating the macro dependencies. It
turns out that the current macro plugin implementation doesn't need the
downstream users to know about the plugin search path from the upstream
dependencies.

Secondly, fix a bug that the swiftinterface that has macro usage won't
build because the build command doesn't inherit the plugin search path
option.

Finally, add JSON output for macro dependencies so it is easier to
debug the macro dependencies.

rdar://131214106
cachemeifyoucan added a commit to cachemeifyoucan/swift that referenced this pull request Jul 8, 2024
Fix few issues from previous implementation from explicit module build
with macros and accurate macro dependency scanning in
swiftlang#73421.

First, there is a crash when propagating the macro dependencies. It
turns out that the current macro plugin implementation doesn't need the
downstream users to know about the plugin search path from the upstream
dependencies.

Secondly, fix a bug that the swiftinterface that has macro usage won't
build because the build command doesn't inherit the plugin search path
option.

Finally, add JSON output for macro dependencies so it is easier to
debug the macro dependencies.

rdar://131214106
cachemeifyoucan added a commit to cachemeifyoucan/swift that referenced this pull request Jul 9, 2024
Fix few issues from previous implementation from explicit module build
with macros and accurate macro dependency scanning in
swiftlang#73421.

First, there is a crash when propagating the macro dependencies. It
turns out that the current macro plugin implementation doesn't need the
downstream users to know about the plugin search path from the upstream
dependencies.

Secondly, fix a bug that the swiftinterface that has macro usage won't
build because the build command doesn't inherit the plugin search path
option.

Finally, add JSON output for macro dependencies so it is easier to
debug the macro dependencies.

rdar://131214106
cachemeifyoucan added a commit to cachemeifyoucan/swift that referenced this pull request Jul 11, 2024
Fix few issues from previous implementation from explicit module build
with macros and accurate macro dependency scanning in
swiftlang#73421.

First, there is a crash when propagating the macro dependencies. It
turns out that the current macro plugin implementation doesn't need the
downstream users to know about the plugin search path from the upstream
dependencies.

Secondly, fix a bug that the swiftinterface that has macro usage won't
build because the build command doesn't inherit the plugin search path
option.

Finally, add JSON output for macro dependencies so it is easier to
debug the macro dependencies.

rdar://131214106
(cherry picked from commit da10a02)
cachemeifyoucan added a commit to cachemeifyoucan/swift that referenced this pull request Jul 15, 2024
Fix few issues from previous implementation from explicit module build
with macros and accurate macro dependency scanning in
swiftlang#73421.

First, there is a crash when propagating the macro dependencies. It
turns out that the current macro plugin implementation doesn't need the
downstream users to know about the plugin search path from the upstream
dependencies.

Secondly, fix a bug that the swiftinterface that has macro usage won't
build because the build command doesn't inherit the plugin search path
option.

Finally, add JSON output for macro dependencies so it is easier to
debug the macro dependencies.

rdar://131214106
(cherry picked from commit da10a02)
cachemeifyoucan added a commit to cachemeifyoucan/swift that referenced this pull request Jul 16, 2024
Fix few issues from previous implementation from explicit module build
with macros and accurate macro dependency scanning in
swiftlang#73421.

First, there is a crash when propagating the macro dependencies. It
turns out that the current macro plugin implementation doesn't need the
downstream users to know about the plugin search path from the upstream
dependencies.

Secondly, fix a bug that the swiftinterface that has macro usage won't
build because the build command doesn't inherit the plugin search path
option.

Finally, add JSON output for macro dependencies so it is easier to
debug the macro dependencies.

rdar://131214106
(cherry picked from commit da10a02)
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.

3 participants