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

flush multiple lines together #204

Merged
merged 5 commits into from
Sep 27, 2023
Merged

flush multiple lines together #204

merged 5 commits into from
Sep 27, 2023

Conversation

graefjk
Copy link
Contributor

@graefjk graefjk commented Sep 26, 2023

Currently every new line is flushed to the console individualy.
Flushing multiple lines together noticeably reduces Lag when many lines are written to the console e.g. when spawning many entities.

flushing multiple lines together noticeably improves performance when
many lines are written to the console e.g. when spawning many entities
buehlefs
buehlefs previously approved these changes Sep 27, 2023
Copy link
Collaborator

@buehlefs buehlefs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe offer max buffer length as a setting via the constructor.

*/
public ConsoleBufferedOutputStream(final JTextPane textPane, final OutputStyle style, int maxBufferLength) {
this(textPane, style);
this.maxBufferLength = maxBufferLength;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably not as effective, as the stringbuilder is already created in the line above.

* maxBufferLength
*/
public void setMaxBufferLength(int maxBufferLength) {
this.maxBufferLength = maxBufferLength;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the max buffer length is longer than the current string builder buffer? (it is probably good to enforce some bounds, also in the constructor)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right setters don't make sense in this case as the Stringbuilder is already build in the constructor and changing maxBufferLenght does actually nothing.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has some effect if you set it to lower values

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the Stringbuilder is already build. I dont think changing maxBufferLength after it is build would change the StringBuilder.
this.lineBuffer = new StringBuilder(maxBufferLength * 2);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the buffer gets flushed immediately if it is >maxBufferLength This would still be effective, even.if the actual buffer size is static

@@ -43,7 +43,8 @@
public class ConsoleBufferedOutputStream extends OutputStream {

/** The maximum length of the line buffer. */
private int maxBufferLength = 4096;
static final int STANDART_MAX_BUFFER_LENGTH = 4096;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEFAULT_MAX_BUFFER_LENGTH

Copy link
Collaborator

@buehlefs buehlefs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave a thumbs up if i should merge this

@buehlefs buehlefs merged commit a8f31dc into master Sep 27, 2023
2 checks passed
@buehlefs buehlefs deleted the limitConsoleWrites branch September 27, 2023 13:15
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.

2 participants