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

Slow print after upgrade #124

Open
neon-dev opened this issue Sep 8, 2021 · 5 comments · May be fixed by #125
Open

Slow print after upgrade #124

neon-dev opened this issue Sep 8, 2021 · 5 comments · May be fixed by #125

Comments

@neon-dev
Copy link
Contributor

neon-dev commented Sep 8, 2021

A user reported slow prints (SerialPrinter) after updating to 2.0. Is there some new setting I need to define?
Note: Previous version was 1.4.0, so 1.6.0 was skipped.
Thanks

@neon-dev
Copy link
Contributor Author

neon-dev commented Sep 9, 2021

I found the reason:

await Task.Delay(100);

Since our data is sent line by line, this delay slows printing down dramatically.

@neon-dev neon-dev linked a pull request Sep 9, 2021 that will close this issue
@igorocampos
Copy link
Collaborator

igorocampos commented Sep 9, 2021

Few more info about this change. It was introduced at 9a68c53 and there was a Delay of 50 ms before in WriteLongRunningTask method, but in a different line.
image

So the said commit actually moved it up, and doubled the waiting

@neon-dev
Copy link
Contributor Author

neon-dev commented Sep 9, 2021

Yes and the 50 ms delay got introduced with 32fae5e but never got published with any version it seems.
v1.6.0 and 1.4.0 (the two most recent versions before 2.0) use the same synchronous code:

public virtual void Write(byte[] bytes)
{
int bytePointer = 0;
int bytesLeft = bytes.Length;
bool hasFlushed = false;
while (bytesLeft > 0)
{
int count = Math.Min(_maxBytesPerWrite, bytesLeft);
Writer.Write(bytes, bytePointer, count);
BytesWrittenSinceLastFlush += count;
if (BytesWrittenSinceLastFlush >= 200)
{
// Immediately trigger a flush before proceeding so the output buffer will not be delayed.
hasFlushed = true;
Flush(null, null);
}
bytePointer += count;
bytesLeft -= count;
}
if (!hasFlushed)
{
FlushTimer?.Start();
}
}

@igorocampos
Copy link
Collaborator

@lukevp do you know why there was a double in the delays?
From commit description is says Refactor delays in main loop so that it's delayed even if things are null

@lukevp
Copy link
Owner

lukevp commented Sep 16, 2021

@igorocampos @neon-dev I'll take a look this weekend and factor this in to the process. Sorry for the issues! I versioned it up to 2.0 since there were major changes to the networking and threading code in this release. Thanks for the PR @neon-dev, I'll probably take in the fix and tweak it slightly. The purpose of the sleeps overall is so that each read/write thread doesn't busy loop (since they are in a tight while loop that never ends, it will peg a CPU core at 100% if there is no deferring or async actions taking place). If there is actually work to be done, there shouldn't be a need for a delay. I'll provide more comments on the PR when I review.

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 a pull request may close this issue.

3 participants