Avoid pathological failure mode when trimming lines#82
Conversation
|
This has been reported by a couple more users as noticably improving Minecraft's performance while using sklauncher. |
If for any reason the document associated with a LimitLinesDocumentListener ended up not getting shorter when its first line was removed (this is, of course, impossible), sklauncher would wedge in a busy loop that completely killed the GUI thread forever. Solution, part SKCraft#1: Don't do a while loop, just remove multiple lines if you need to. But wait, why are we having this problem at all? It turns out that there are problems when the document's length changes, which can manifest as tons of BadLocationExceptions going uncaught in the AWT thread which is trying to refresh the window. Why? Probably because the insert into the document isn't happening in the Swing run queue, but concurrently with it. So use an invokeLater to do the actual insertion. With these two changes, and a console set to 125 lines, I got no failures. Previously, if I set the console to 125 lines, it would die catastrophically before Minecraft was done starting up. Also performance may be noticably better. It's pretty common for things to dump 20+ lines into the buffer at once. If you have a 10,000 line buffer, and you then delete the first line of it 20 times, that's actually a whole lot of allocation and shuffling going on.
|
I do not understand everything here (it's too complicated for me ^^'). |
|
It is possible for the console to get into a state (possibly only on Windows?) where the console locks up completely, the launcher cannot refresh, the launcher uses 100% of a core of CPU time, and the Minecraft client is completely hosed because it has horrible problems every time it tries to write any output to its output streams. The specific issue is that the client ends up in a state where removeFromStart() does not actually reduce the number returned by getElementCount(), meaning the while loop becomes an infinite loop. This appears to be caused by a bug which is only exposed if the inserts collide with the rendering loop trying to access the same data, which causes an exception. But it also appears that the bug only shows up when you have \r in the text, probably. Then once you get that fixed, there's a performance problem in that the launcher is spending a ridiculous amount of time on trimming strings. Consider what happens when an addon dumps a 31-line message to the log and the log is at 10k lines. The log is now at 10,031 lines. removeLines fires. It checks the line count, determines that it's high, calls removeFromStart(), which finds the length of the first line, and removes those characters. Now we're at 10,030 lines... And it has to do this 30 more times. And they may or may not involve allocations and copies, but there's certainly a lot of crap happening. (Even assuming we use the gap implementation for the document, the gap is getting slammed back and forth between end for the appends and beginning for the removes.) I honestly have no idea why the original implementation did a while loop there. |
|
Waouw, OK ^_^ |
|
I've noticed that lockup. I'll look into this PR in the coming days. |
Avoid pathological failure mode when trimming lines.
|
Thanks for the PR. |
3 participants
If for any reason the document associated with a LimitLinesDocumentListener
ended up not getting shorter when its first line was removed (this is,
of course, impossible), sklauncher would wedge in a busy loop that
completely killed the GUI thread forever.
Solution, part #1: Don't do a while loop, just remove multiple lines
if you need to.
But wait, why are we having this problem at all? It turns out that there
are problems when the document's length changes, which can manifest as
tons of BadLocationExceptions going uncaught in the AWT thread which is
trying to refresh the window. Why? Probably because the insert into the
document isn't happening in the Swing run queue, but concurrently with
it. So use an invokeLater to do the actual insertion.
With these two changes, and a console set to 125 lines, I got no failures.
Previously, if I set the console to 125 lines, it would die catastrophically
before Minecraft was done starting up.
Also performance may be noticably better. It's pretty common for things
to dump 20+ lines into the buffer at once. If you have a 10,000 line buffer,
and you then delete the first line of it 20 times, that's actually a whole
lot of allocation and shuffling going on.
Much thanks to "kenzierocks" from IRC for helping debug this!