Skip to content
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

bug 8858 - Switch to analyze dependencies without printing them #7400

Closed
wants to merge 1 commit into from

Conversation

marler8997
Copy link
Contributor

…or to a file.

This is in response to (https://issues.dlang.org/show_bug.cgi?id=8858). It allows the option -deps- which causes the compiler to analyze dependencies without printing them to stderr or to a file. The typical use case for this would be when a tool like rdmd needs to know all the dependencies but is using the verbose output instead of the dependency specific output.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @marler8997! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

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.

@marler8997 marler8997 changed the title Allow loading/analyzing dependencies without printing them to stderr … bug 8858 - Switch to analyze dependencies without printing them Dec 6, 2017
@andralex
Copy link
Member

andralex commented Dec 7, 2017

This seems like an inflation of options. What's wrong with directing deps to /dev/null? Is outputting deps a bottleneck?

We really need to stop defining new artifacts that are subtly different versions of existing artifacts. The right way to go is improve the existing artifacts.

@marler8997
Copy link
Contributor Author

Even if output goes to /dev/null (which I don't how you would do that on windows) dmd is still spending cycles generating the output. I estimate this is a very minor performance savings but I thought I would do what I could to try to improve the performance of rdmd in any way I can. Weighing the simplicity of this change with a minor performance savings in rdmd seemed like it could be worth it, but I could go either way on this one. Based on your comments about how this complicates dmd's interface I'll go ahead and close this one.

@marler8997 marler8997 closed this Dec 7, 2017
@andralex
Copy link
Member

andralex commented Dec 8, 2017

I estimate this is a very minor performance savings but I thought I would do what I could to try to improve the performance of rdmd in any way I can.

The one way to go about this is to measure.

@marler8997
Copy link
Contributor Author

For the digger project, on windows (piping output to a file, which is faster than going to the console) it saves about 100 ms out of about 8 seconds. Around a 1 to 2% savings.

@DmitryOlshansky
Copy link
Member

DmitryOlshansky commented Dec 8, 2017 via email

@rainers
Copy link
Member

rainers commented Dec 8, 2017

Even if output goes to /dev/null (which I don't how you would do that on windows)

FYI: the corresponding file on Windows is NUL:.

@marler8997
Copy link
Contributor Author

Didn't know about NUL:. Piping commands to NUL: seems to work but it doesn't seem to work as a filename passed to -deps. I tried -deps=NUL and -deps=NUL: but I just get Error writing to file 'NUL:'.

@rainers
Copy link
Member

rainers commented Dec 8, 2017

I tried -deps=NUL and -deps=NUL: but I just get Error writing to file 'NUL:'.

Both work for me with the released dmd, but not if dmd is built with VC.

@rainers
Copy link
Member

rainers commented Dec 8, 2017

I suspect it's not VC related, but caused by #7299

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants