ESC-POS-.NET icon indicating copy to clipboard operation
ESC-POS-.NET copied to clipboard

Fix #124 (slow prints when data is sent in chunks)

Open neon-dev opened this issue 3 years ago • 4 comments

Should fix #124 but untested. Maybe ReadLongRunningTask() should get the same treatment, not sure.

neon-dev avatar Sep 09 '21 14:09 neon-dev

Hmm, after some googling a BlockingCollection should be the better solution as it works without arbitrary delays. I'm not very experienced in C#, so sorry this fix isn't ideal.

neon-dev avatar Sep 09 '21 14:09 neon-dev

I haven't reviewed yet but I hope to do so this weekend. In the meantime, @neon-dev would you mind adding a new test type to the console app as part of this PR that reproduces your issue? If I understand correctly, the issue is because there's a long receipt with each line being a separate printer.Write() call instead of using a ByteSplicer.Combine() to send a single write call? Would be good to have an example in the console test suite since we typically run through those before publishing a new version and would make sure we don't have a performance regression here in the future.

lukevp avatar Sep 16 '21 04:09 lukevp

I counted how many individual Write calls the shortest possible receipt takes (including commands for alignment etc.) and got 45. So the old synchronous code can be tested with these simple lines and results in ~31 ms:

var stopwatch = new Stopwatch();
stopwatch.Start();
for (int i = 0; i < 45; i++) {
    printer.Write(E.LeftAlign());
}
stopwatch.Stop();
Console.WriteLine($"Duration: {stopwatch.ElapsedMilliseconds} ms");

But the new asynchronous code will report 0 ms and can only be measured by modifying the BasePrinter class and adding a(nother) wait logic somewhere to detect when the WriteBuffer is fully drained. I don't know about you but I dislike this idea.

We wouldn't have to worry about regressions if the async task would use said BlockingCollection backed by a ConcurrentQueue because then there are no delays involved, which would make everything even more responsive while also removing the potential for CPU load issues. Unfortunately I won't find enough time in the near future to update my PR with this superior, alternative solution.

neon-dev avatar Sep 16 '21 08:09 neon-dev

@lukevp have you looked into a proper solution? I'd also suggest a note about this performance issue in the README in the meantime.

neon-dev avatar Jul 31 '22 01:07 neon-dev