Skip to content

[Driver] <rdar://43955209> Remove obsolete/fragile batch mode 'repartitioning' code. #19802

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

Conversation

graydon
Copy link
Contributor

@graydon graydon commented Oct 9, 2018

Remove the batch-mode support for repartitioning the input batches to try to fit inside argv limits.

This code is obsolete / dead because:

  • Repartitioning was only subdividing down the batch partition which isn't able to minimize overlong argv that could happen in (say) the list of all inputs. So the "remedy" it was applying wouldn't always work.
  • With support for supplementary output file maps, any invocation with more than 8 primaries was going to trigger the use of a supplementary output file map anyways (limit of 128 divided by 16 filetypes), leaving only the rare chance of executing on a platform with extremely small argv (POSIX sets minimum at 4096 bytes, but it's 32kb on win32 and over 200kb on macosx) exceeded by fewer-than-8 filenames.
  • Subsequent support for response files uses the same sysconf-based measurement to make even this case not possible anyways: it's caught and remedied correctly right at the end of command-line construction.

Given the dead-code-ness and the fact that the test for it "working" fails sporadically causing endless annoyance to czars, I propose dropping it.

rdar://43955209

@graydon graydon requested review from akyrtzi and davidungar October 9, 2018 20:45
@graydon
Copy link
Contributor Author

graydon commented Oct 9, 2018

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Oct 9, 2018

Build failed
Swift Test Linux Platform
Git Sha - 0f26b9c

Copy link
Contributor

@davidungar davidungar left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks in advance for adding the assertion to ensure that response files are always allowed between driver and frontend.

@graydon
Copy link
Contributor Author

graydon commented Oct 9, 2018

@swift-ci please test and merge

@davidungar
Copy link
Contributor

Nice comment on the assertion!

@graydon
Copy link
Contributor Author

graydon commented Oct 9, 2018

@swift-ci please test linux platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 0f26b9c

@graydon
Copy link
Contributor Author

graydon commented Oct 10, 2018

@swift-ci please test macos platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 0f26b9c

@graydon graydon merged commit f05be88 into swiftlang:master Oct 10, 2018
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