Rewrite without slice-deque (assertion failed at src\reader.rs:553:13 when running example)

[Sopel97]

I wanted to benchmark this against my implementation to have a baseline and I hit problems when running the examples.
running on windows
rustc 1.38.0 (625451e37 2019-09-23)
cargo 1.38.0 (23ef9a4ef 2019-08-20)

I compile with cargo build --examples --release
Then I run with cargo run --release --example validate -- "c:/dev/chess_pos_db/data/lichess_db_standard_rated_2014-12.pgn"
And I get an error

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `65536`,
 right: `16384`', src\reader.rs:553:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
error: process didn't exit successfully: `target\release\examples\validate.exe c
:/dev/chess_pos_db/data/lichess_db_standard_rated_2014-12.pgn` (exit code: 101)

All examples have the same issue. I also tried running the binary directly but no difference. Looks like the buffer is not being filled properly? My knowledge of rust is literally 0 so I can't do much here.
I also tried increasing the MIN_BUFFER_SIZE 4 times, then the assertion passes but the program hangs, no disk read happens at all.

[niklasf]

Thanks for reporting. It looks like I am making (and asserting) some assumptions that do not hold on Windows.

It will be a while (maybe a week) until I'll have a chance to debug this. I'll also make sure to add CI for Windows.

[Sopel97]

Any progress on this? I would appreciate even any ideas/pointers as to what may be the issue here.

[niklasf]

Ok, so it looks like we can simply relax the assertion in question.

There is a related remaining soundness issue, because calling tail_head_slice() on a not-fully initialized SliceDeque might be insta UB. I'll try to figure that out before making a release.

[Sopel97]

Thanks. Now it seems to be working. Though there is still something wrong. It takes a few minutes for a ~100MiB file (lichess 2013-01). I'm doing exactly what I did before. stats runs just as slow. Full cpu core is being used. looking at disk usage it's 50% reads 50% nothing. Totalling around 300KiB/s average.
Can you reproduce it on windows?

[niklasf]

Wow, that's horrible. It'll probably be (at least) another week before I can get my hands on a Windows machine again.

[Sopel97]

No worry, it's not urgent.

[Eugenio-Bruno]

Hi,

I'm running into the same problem. Sorry to nag you about this, but any progress or workaround?

[niklasf]

No, sorry. I think a proper fix might require a rewrite with an entirely different approach. I want to do that, but no promises on the timeline.

[thesw4rm]

Hi, I'd like to also mention the issue of using SliceDeque. Current issue I'm facing is getting it to work with websockets, because SliceDeque is built to modify memory directly which is kinda hard to do when your code is essentially getting embedded in a web browser.

Code looks not terribly written (thank you), but can you give any insight into why the SliceDeque it is needed for parsing the PGN? I only found one instance of it being defined here

/// Internal read ahead buffer.
#[derive(Debug, Clone)]
pub struct Buffer {
    inner: SliceDeque<u8>,
}

in parse.rs

Everything else is just defining as a generic type.

From looking at the usage, looks like you're just using it to incrementally read in larger buffers from the PGN file. Can you a bit more into why SliceDeque instead of something like simple vec! before I start delving into changing stuff and of course pull requesting this back once I'm done? Would really appreciate it!

[niklasf]

The idea was that it's very efficient as a ring buffer. Let's say the buffer content is

|_______________ABCDEF|

where _ is some old/invalid/uninitialized content. The tag or whatever is not complete yet, so we have to rotate

|ABCDEF_______________|

and can then continue to read into the buffer

|ABCDEFGH_____________|

With SliceDeque this is O(1), with Vec moving the contents to the start of the buffer is O(n). But whether or not it is really fast, is unfortunately platform specific (which I didn't realize when implementing this), and some assumptions about SliceDeque internals (like the assertion) are also fragile.