Use whitelist to find go files, run find only once #10594
Conversation
|
@guillep2k this should solve what you mentioned yesterday. What's interesting is that even if I exclude the bindata files from |
Codecov Report
@@ Coverage Diff @@
## master #10594 +/- ##
==========================================
- Coverage 43.62% 43.6% -0.03%
==========================================
Files 588 588
Lines 82474 82483 +9
==========================================
- Hits 35980 35966 -14
- Misses 42029 42055 +26
+ Partials 4465 4462 -3
Continue to review full report at Codecov.
|
4420ec0 to
de9e8d7
Compare
It's because the go compiler compiles the whole package. Including them in the source list is only needed for change detection. |
|
One more potential point for optimization is |
I don't think we need to have it dynamic (but keep on reading). AFAIU, the only reason for keeping it "dynamic" would be to support a forward reference. Consider the following: Here I don't think that's the situation with Having said that, |
|
Right, |
|
@guillep2k your example contains the anti-pattern of using a variable before it was defined. I would not allow such usage, even with lazy evaluation. Maybe there are linters for Makefiles that could catch such anti-patterns, a quick search leads me to https://github.com/mrtazz/checkmake. |
It was just a (lame) example, but quite a useful pattern in make, because then you've got a find functionality and many places where you can use it in different scenarios. We're using only one Makefile, but there are build systems with 10s or 100s that chain up and down. As with C++, you will need "foward declarations" at some point. 😄 |
dbfaad9 to
661129a
Compare
|
@jolheiser it seems your review was requested. Would you mind? |
cc53fe1 to
539f8cf
Compare
|
Is this error coming from the PR itself or is it a glitch? https://drone.gitea.io/go-gitea/gitea/22479/2/10 Although it only failed in MySQL builds, sounds Makefiley to me. I'm restarting the CI just in case. |
|
|
|
Added contrib which should get rid of these warnings. |
|
Hmm that didn't do anything to the warnings. The warnings don't seem come from this PR. I see them on https://drone.gitea.io/go-gitea/gitea/22489/2/10 too.
Should I remove it again? |
|
I don't think |
|
ok, reverted |
- Use a whitelist-based approach to find *.go files so we can use standard find syntax. Also included is a change that files no longer pass a leading './' which should help performance further. - Instead of running `find` multiple times in make because of the lazy evaluation, run it only once and add the bindata files when the bindata tag is present This is another huge speedup on my machine of around 2000%.
4f4d5a5 to
b8a2444
Compare
|
rebased, good to go |
|
Ping LG-TM |
Use a whitelist-based approach to find *.go files so we can use standard find syntax. Also included is a change that files no longer pass a leading './' which should help performance further.
Instead of running
findmultiple times in make because of the lazy evaluation, run it only once and add the bindata files when the bindata tag is presentThis is another huge speedup on my machine of around 2000%.