Skip to content

Compile dmd exe in build.d#10446

Merged
dlang-bot merged 1 commit intodlang:masterfrom
marler8997:compileDmdBuildD
Oct 3, 2019
Merged

Compile dmd exe in build.d#10446
dlang-bot merged 1 commit intodlang:masterfrom
marler8997:compileDmdBuildD

Conversation

@marler8997
Copy link
Contributor

@marler8997 marler8997 commented Oct 2, 2019

Move compiling the main dmd executable into build.d.

@marler8997 marler8997 marked this pull request as ready for review October 2, 2019 17:08
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @marler8997! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#10446"

@thewilsonator
Copy link
Contributor

Failed to download package eventcore from https://code.dlang.org/packages/eventcore/0.8.35.zip

You'll need to give it another force push.

@dlang-bot dlang-bot merged commit 6bb95b5 into dlang:master Oct 3, 2019
@WalterBright
Copy link
Member

I don't know how build.d works, but it appears to simply grab all the .d files in the directory. This causes it to fail on my machine, as I tend to leave around other .d files, and my editor makes backup files by prepending .B to the filename.

@wilzbach
Copy link
Contributor

With this PR the CodeCov coverage report dropped to 1.8%:

https://codecov.io/gh/dlang/dmd/tree/6bb95b5bed514070e13ec9a497f47a80e8dfc5c7

@marler8997
Copy link
Contributor Author

Sounds like we are missing -cov flag somewhere

@marler8997
Copy link
Contributor Author

After doing some digging, I think the problem could be that we are now passing in absolute paths for the filenames. I also had to convert to relative names to fix doc generation in #10516

@marler8997
Copy link
Contributor Author

fix here: #10520

@CyberShadow
Copy link
Member

@marler8997 This change caused the file size of the dmd binary to go up by 1 MB:

https://perf.dlang.io/#size-dmd;size-dmd;1570028973.2662013;1570217574.2034426

@MoonlightSentinel
Copy link
Contributor

Guess this difference is caused by ‘build.d‘ including debug symbols unless ‘ENABLE_RELEASE‘ is specified

@CyberShadow
Copy link
Member

Seems doubtful, the difference between a debug and release build is rarely just 10% of the binary size, usually it's closer to an order of magnitude.

@rainers
Copy link
Member

rainers commented Dec 16, 2019

Could it be that perf.dlang.io builds the default target, not release? According to this change https://perf.dlang.io/#size-dmd;size-dmd;1525732319.848405;1526707174.5968058 enabling debug information for that added abaout 3 MB.

@marler8997
Copy link
Contributor Author

@CyberShadow thanks for finding this. I'll try to see what's going on here.

@marler8997
Copy link
Contributor Author

@CyberShadow where is this dmd binary coming from that your TrenD tool is analyzing?

@marler8997
Copy link
Contributor Author

marler8997 commented Dec 16, 2019

Looks like the reason for the change in size is that build.d is compiling a slightly different set of source files than the makefiles are. All other command-line arguments are identical.

posix.mak had a couple extra files dmd/id.d and dmd/root/region.d.

build.d had these extra files: dmd/astbase.d, dmd/libmscoff.d, dmd/libomf.d, dmd/scanmscoff.d, dmd/scanomf.d and dmd/strictvisitor.d.

When I removed one of the extra files dmd/astbase.d it removed about a megabyte from the dmd binary.

I think enabling LTO would solve any issues with including extra source files that aren't being used. We could also go through the current build to see if dmd is including any source files that aren't being used and remove them. At the time this PR was integrated, build.d was listing directories to find source files, now we use explicit filenames in build.d so we can easily control which files to include or exclude.

@marler8997
Copy link
Contributor Author

I've created a PR to remove the new astbase.d and strictvisitor.d modules from dmd. It brought the size down by about a megabyte. #10674

@marler8997 marler8997 deleted the compileDmdBuildD branch December 16, 2019 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants