Optimization for include resolving
ClosedPublic
Actions

Authored by vladislavbelov on Jan 20 2021, 11:26 PM.

Details

Reviewers

None

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP24740: Optimization for include resolving in PreprocessorWrapper.

Summary

Subj, increasing parsing speed up to x10.

Test Plan

Apply the patch and compile the game
Run tests: test TestPreprocessor, make sure all passed
Check that all included effects work as before: fog, shadows, los

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vladislavbelov created this revision.Jan 20 2021, 11:26 PM

Build has FAILED

builderr-debug-macos.txt
../../../source/graphics/ShaderProgram.cpp:86:15: warning: 'Reload' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
        virtual void Reload()
                     ^
../../../source/graphics/ShaderProgram.h:124:15: note: overridden virtual function is here
        virtual void Reload() = 0;
                     ^
../../../source/graphics/ShaderProgram.cpp:118:15: warning: 'Bind' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
        virtual void Bind()
                     ^
../../../source/graphics/ShaderProgram.h:135:15: note: overridden virtual function is here
        virtual void Bind() = 0;
                     ^
../../../source/graphics/ShaderProgram.cpp:128:15: warning: 'Unbind' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
        virtual void Unbind()
                     ^
../../../source/graphics/ShaderProgram.h:140:

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/2999/display/redirect
See console output for more information: https://jenkins.wildfiregames.com/job/macos-differential/2999/display/redirectconsole

Harbormaster completed remote builds in B15307: Diff 15589.Jan 20 2021, 11:39 PM

vladislavbelov requested review of this revision.Jan 20 2021, 11:39 PM

Stan added a subscriber: Stan.Jan 20 2021, 11:52 PM

Stan added inline comments.

source/graphics/PreprocessorWrapper.cpp
111 ↗	(On Diff #15589)	Should probably include `unordered_map`
140 ↗	(On Diff #15589)	auto?
236 ↗	(On Diff #15589)	Why the std:: prefix, first time I see it?

Seems to work, go from 0.21 to .16

In D3030#145966, @Stan wrote:

The tests run 4 times faster on my Kubuntu (I5-4300U) (0.058040s) than on my Windows (I7-8750H) (0.214631s)

vladislavbelov added inline comments.Jan 20 2021, 11:59 PM

source/graphics/PreprocessorWrapper.cpp
111 ↗	(On Diff #15589)	Already included in header.
140 ↗	(On Diff #15589)	It was `auto` :)
236 ↗	(On Diff #15589)	Actually a habbit, because by standard it's not guaranteed that there is `size_t` without including a C-header.