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

Another max_piece_type() cleanup. #81

Closed
wants to merge 1 commit into from

Conversation

mstembera
Copy link
Contributor

Since this is already getting the pedantic treatment here is my submission.

Timings using AMD Phenom II, gcc 4.9.1, build ARCH=x86-64-modern
start /B /REALTIME /AFFINITY 0x1 stockfish.exe bench 1>nul

Master
4134
4134
4134
4134
4134 (yes I really did get exactly 4134 5 times in a row)

Patch
4119
4119
4118
4118
4118

Very minor speedup but it also removes a line.

@bunkbail
Copy link

Fishbench output:

Results for 50 tests for each version:

        Base      Test      Diff      
Mean    2029859   2035143   -5284     
StDev   22261     21292     7421      

p-value: 0.762

0.26% speedup. Insignificant speedup, but still a speedup :)

@mcostalba
Copy link

I was thinking to something similar this early morning :-)

Yes, declaring PieceType pt inside the loop scope should help the compiler.

@zamar
Copy link

zamar commented Oct 28, 2014

Approved

@lucasart
Copy link

Yes, scoping variables as tightly as possible is best practice. Unfortunately a lot of Stockfish code does not follow.this principle, and uses the old C89 logic of declaring all local variables at the beginning of a function.

@mcostalba
Copy link

It is not always the faster solution (tested)...but in this case it is.

On Tue, Oct 28, 2014 at 10:09 AM, lucasart notifications@github.com wrote:

Yes, scoping variables as tightly as possible is best practice.
Unfortunately a lot of Stockfish code does not follow.this principle, and
uses the old C89 logic of declaring all local variables at the beginning of
a function.


Reply to this email directly or view it on GitHub
#81 (comment)
.

@lucasart
Copy link

I was thinking of coding style, and restricting scope of variables means when you read a piece of code you have less variables to think about. The more local variables the harder the code to understand. I don't think of it as a speedup trick, and I'm really not a fan of micro optimization.

@mcostalba
Copy link

In the past I attempted to move locals definition away from function top,
just for coding style reasons (you know I am a bit paranoid on this :-) ,
but I didn't get a more readable code, actually specifying also the type in
an already quite long statement did not help and in many cases result was
less readable. At least this was my understanding after my failed attempt.

On Tue, Oct 28, 2014 at 12:24 PM, lucasart notifications@github.com wrote:

I was thinking of coding style, and restricting scope of variables means
when you read a piece of code you have less variables to think about. The
more local variables the harder the code to understand. I don't think of it
as a speedup trick, and I'm really not a fan of micro optimization.


Reply to this email directly or view it on GitHub
#81 (comment)
.

@lucasart
Copy link

I would blame the long statements then. You know… these complex one liner formulas in SF that could simply be broken in two (adding a line of code for readability sake). what do you think?

@glinscott glinscott closed this in 5605cc7 Oct 28, 2014
@glinscott
Copy link
Contributor

Approved!

niklasf pushed a commit to niklasf/Stockfish that referenced this pull request Oct 31, 2016
Skip tablebase probing in crazyhouse
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.

None yet

6 participants