Skip to content

Conversation

@alexandrumc
Copy link
Contributor

There's an unnecessary semantic analysis done on templates from imported modules which leads to a compilation that takes up to 5x longer.

module foo;

import std.uni;

Before

time dmd -c foo.d 

real	0m0,128s
user	0m0,121s
sys	0m0,008s

time dmd -c -unittest foo.d

real	0m0,663s
user	0m0,592s
sys	0m0,072s

After:

time dmd -c -unittest foo.d 

real	0m0,149s
user	0m0,138s
sys	0m0,012s

Related to: #8124

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @alexandrumc! 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#10641"

@alexandrumc
Copy link
Contributor Author

@atilaneves

@alexandrumc alexandrumc changed the title Remove useless deferred semantic analysis 2 and 3 done when using -unittest flag Remove deferred semantic analysis 2 and 3 done when using -unittest flag Dec 6, 2019
@thewilsonator
Copy link
Contributor

The build kite dscanner failure looks unrelated.

@rainers
Copy link
Member

rainers commented Dec 7, 2019

The build kite dscanner failure looks unrelated.

It does for me, because std.format has unittests in templates, e.g.
https://github.com/dlang/phobos/blob/master/std/format.d#L1222 and https://github.com/dlang/phobos/blob/master/std/format.d#L1578. (Side note: The first one is pretty annoying and confusing when running your program in a debugger: it might stop execution because of the exception thrown in the unittest even though you don't have any in your own code. It's also rather pointless to rebuild these unittests everytime if they don't use the template arguments.)

I suspect that there is some code generation missing for unittests inside templates with this PR.

@alexandrumc alexandrumc closed this Jan 2, 2020
@alexandrumc alexandrumc reopened this Jan 2, 2020
@RazvanN7
Copy link
Contributor

What's the status on this? @alexandrumc were you able to find the root of the failure in dscanner?

@alexandrumc
Copy link
Contributor Author

What's the status on this? @alexandrumc were you able to find the root of the failure in dscanner?

The work has moved here: #10759

@thewilsonator
Copy link
Contributor

So should this be closed?

@alexandrumc
Copy link
Contributor Author

So should this be closed?

Yes, it should.

@atilaneves atilaneves closed this Feb 19, 2020
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.

7 participants