Perft speed optimization (renamed)

[mvanthoor]

Got another 1.5% on my machine, and also tested well for real games

So what did you change to gain this 1.5%? In your last commit, I see a few changes that shouldn't have any effect. You do seem to have moved the switch that moves the rook. That could be the biggest change. It could actually be that it's not really faster because it's faster, but because the switch is now in a different place. Still, with another 1.5%, you could possibly hit 200 seconds (+/- 0.5) on KiwiPete6 on my machine.

I had another place (in unmake()) where I could use the en-passant to ^ 8 trick, but it slowed things down. Reversing the to ^ 8 trick in make() also slows things down :p

BTW; I've sent an invitation through github so you can become a collaborator on Rustic; that way you can get a sneak peek at the engine before it's actually released.

[Terje]

Got another 1.5% on my machine, and also tested well for real games

So what did you change to gain this 1.5%?

I made the en passant square be 0 (A1) when not set, rather than NO_SQ (99), and made the en passant hash key for A1 be 0 (A1-H8 had unique en pas keys before). With this I can now unconditionally hash the en passant square out, rather than checking to make sure it's a valid square first. It also makes calling the Fathom api cleaner as it expects 0 for no en pas, as well as the couple of if's still left being cleaner.

Removed the 5th path from the castling switches - a castling move will always have one of the 4 destinations I listed, so I made the 4th one the default.

And finally moved the castling switch in makemove to not happen for pawn moves.

If the branch predictor could get it right all the time, these changes would probably end up negative, and they might not help for other engines, but they worked for Weiss

And thanks, I'll have a peek!

[mvanthoor]

I made the en passant square be 0 (A1) when not set, rather than NO_SQ (99), and made the en passant hash key for A1 be 0 (A1-H8 had unique en pas keys before). With this I can now unconditionally hash the en passant square out, rather than checking to make sure it's a valid square first.

My EP-square is an Option<T>, which is Rust's concept of "possibly NULL". An Option<T> is always Some(T), or None, and you have to handle both cases. I removed it to get rid of the check and just use A1 = 0 as you did, but stuff became slower.

Removed the 5th path from the castling switches - a castling move will always have one of the 4 destinations I listed, so I made the 4th one the default.

I can't "to" is a Square (which secretly is just an alias for usize/u64), it can have more than the four values of the to-squares while castling. Therefore I must have a match for all other values. (Rust always, everywhere, FORCES to handle all possible cases.)

Code: Select all

match to {
	G1 => ...
	C1 => ...
	G8 => ...
	E8 => ...
	_ => () // do nothing
}

And finally moved the castling switch in makemove to not happen for pawn moves.

And thanks, I'll have a peek!

In the profiler, the castling switch only takes 0.003 seconds of the 78.1 seconds of the run, when doing StartPos7. It is already covered by "if castling {...}". I could try to move it into "if piece != Pieces::PAWN {...}", but I don't think it matters much, because it only takes 0.003 seconds as it is.

[Terje]

I made the en passant square be 0 (A1) when not set, rather than NO_SQ (99), and made the en passant hash key for A1 be 0 (A1-H8 had unique en pas keys before). With this I can now unconditionally hash the en passant square out, rather than checking to make sure it's a valid square first.

My EP-square is an Option<T>, which is Rust's concept of "possibly NULL". An Option<T> is always Some(T), or None, and you have to handle both cases. I removed it to get rid of the check and just use A1 = 0 as you did, but stuff became slower.

Removed the 5th path from the castling switches - a castling move will always have one of the 4 destinations I listed, so I made the 4th one the default.

I can't "to" is a Square (which secretly is just an alias for usize/u64), it can have more than the four values of the to-squares while castling. Therefore I must have a match for all other values. (Rust always, everywhere, FORCES to handle all possible cases.)

Code: Select all

match to {
	G1 => ...
	C1 => ...
	G8 => ...
	E8 => ...
	_ => () // do nothing
}

And finally moved the castling switch in makemove to not happen for pawn moves.

And thanks, I'll have a peek!

In the profiler, the castling switch only takes 0.003 seconds of the 78.1 seconds of the run, when doing StartPos7. It is already covered by "if castling {...}". I could try to move it into "if piece != Pieces::PAWN {...}", but I don't think it matters much, because it only takes 0.003 seconds as it is.

For the second one, sure you can:

Code: Select all

match to {
	G1 => ...
	C1 => ...
	G8 => ...
	_  => do C8 path
}

[mvanthoor]

Of course, but conceptually, that's ugly. It also doesn't gain any speed.

The do nothing part is actually a panic, because to king's to square can't be anything else than one of those 4 when castling.

When you look through the repository, you can also see me jackassing around (sometimes back and forth) trying to decide how to so things in Rust.

Sometimes, I decide to just do it C-style because the Rust way is too convoluted. (Enums for pieces or squares.... Now I use a struct with associated consts as if its a C enum, because actual Rust enums are too verbose for what I want to do.)

[Terje]

Of course, but conceptually, that's ugly. It also doesn't gain any speed.

The do nothing part is actually a panic, because to king's to square can't be anything else than one of those 4 when castling.

When you look through the repository, you can also see me jackassing around (sometimes back and forth) trying to decide how to so things in Rust.

Sometimes, I decide to just do it C-style because the Rust way is too convoluted. (Enums for pieces or squares.... Now I use a struct with associated consts as if its a C enum, because actual Rust enums are too verbose for what I want to do.)

I agree it's not pretty, but having a 5th path when only 4 are possible isn't pretty either in my opinion. I think it's possible to signal that a path isn't possible with __builtin_unreachable() or some such, but I'd rather just do it like I did

I know the feeling going back and forth, there certainly are examples where I've cleaned up code only to clean it up more or less back to the original

[mvanthoor]

I agree it's not pretty, but having a 5th path when only 4 are possible isn't pretty either in my opinion. I think it's possible to signal that a path isn't possible with __builtin_unreachable() or some such, but I'd rather just do it like I did

There's a difference though.

Code: Select all

// x is a char, and SHOULD only be A or B, but COULD be any other char if your program has a bug.
match x {
	'A' => {...}
	'B' => {...}
	_ => { panic! }
}

In this case, if x has anything else but A or B, the program panics and you know you have a bug. If you do it your way, the program executes the "B" path for anything but "A", and you might not notice the bug until the engine either fails or does weird stuff; and such bugs are hard to find.

This is exactly the reason why Rust forces to handle every possible option, and it's not idiomatic to use the last correct option as "last and everything else". (A few days ago, I refactored a bit of code that ended up like this; I'll have to "unrefactor" it.) The idiomatic way in Rust is to never gloss over errors; in most part of the language, if you use it correctly, that isn't even possible.

I know the feeling going back and forth, there certainly are examples where I've cleaned up code only to clean it up more or less back to the original

LOL. Guilty... Yesterday I fixed a "potential bug" that actually isn't a bug. The "fix"does exactly the same thing, but in a different way. In other words: stop overthinking stuff.

[Terje]

I agree it's not pretty, but having a 5th path when only 4 are possible isn't pretty either in my opinion. I think it's possible to signal that a path isn't possible with __builtin_unreachable() or some such, but I'd rather just do it like I did

There's a difference though.

Code: Select all

// x is a char, and SHOULD only be A or B, but COULD be any other char if your program has a bug.
match x {
	'A' => {...}
	'B' => {...}
	_ => { panic! }
}

In this case, if x has anything else but A or B, the program panics and you know you have a bug. If you do it your way, the program executes the "B" path for anything but "A", and you might not notice the bug until the engine either fails or does weird stuff; and such bugs are hard to find.

This is exactly the reason why Rust forces to handle every possible option, and it's not idiomatic to use the last correct option as "last and everything else". (A few days ago, I refactored a bit of code that ended up like this; I'll have to "unrefactor" it.) The idiomatic way in Rust is to never gloss over errors; in most part of the language, if you use it correctly, that isn't even possible.

...

There is a difference, and I'm basically just assuming it works correctly (which I am fairly certain of, atleast for now). Probably not recommended for new engines (unless employing very thorough testing) though, and the code style is definitely questionable. But it gave 0.3MLPS for perft so what's a guy to do haha

[mvanthoor]

It made the engine faster?!

"Coding principles... meet window.*



I'll see if this gains something; I'll also fold the rook capture into castle_perms, and try the to ^ 8 trick in unmake. (All at once; individually they make no discernable difference.)

[Terje]

It made the engine faster?!

"Coding principles... meet window.*



I'll see if this gains something; I'll also fold the rook capture into castle_perms, and try the to ^ 8 trick in unmake. (All at once; individually they make no discernable difference.)

The best solution for speed (having only 4 branches should be a miniscule bit better than 5), and for safety (I can't actually guarantee I never mess up and let a castling move with a wrong destination be inserted into the system in some way), is probably to use an assert in the default path so it "panics" in debug mode if the destination isn't the expected one. However that would look ugly