Skip to content

PSX console window optimization - #37

Open
theandylee wants to merge 1 commit into
psxsplash:mainfrom
theandylee:console-window-optimization
Open

theandylee wants to merge 1 commit into
psxsplash:mainfrom
theandylee:console-window-optimization

Conversation

@theandylee

Copy link
Copy Markdown

I ran my build multiple times, filling the console, and noticed that when I resize the window it hangs for a few seconds during repaint.
Two things I wanted to avoid here:

  • ToArray() on every repaint - pretty huge allocation
  • RemoveRange() from the beginning of the list - moves all subsequent elements to the beginning
    Messages are now stored in a ring buffer and the snapshot is not a newly allocated array, but just a couple of indices.

PS I use a coding agent, but I'm fully humanly responsible for all the stuff I push
PPS I wanted to check out the contrubuting guide, but it's 404 :)

@rixnobis rixnobis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is one real problem here, and I'm not sure it fixes the hang.

The old _snapshot was a copy, so Layout and Repaint drew the same lines whatever the readers did in between. The draw range here is only indices into the live buffer, read outside _lock. Once the buffer is full, every new line overwrites the oldest slot, which is inside [_drawBase, _drawBase + _drawCount). So between Layout and Repaint the lines under the range change. With stdout or stderr hidden, the continue filters then skip a different number of lines in each pass. That brings back the "Getting control position in a group with only N controls" error the snapshot was added to prevent. LogLine is also read there without the lock, so a copy can mix fields from two lines. Selection and "Copy this line" drift the same way: once the buffer is full, their indices move on every new line.

On the hang itself: ToArray of 2000 small structs is about 50 KB, and RemoveRange runs once per 500 lines. Neither should cost seconds. Laying out 2000 GUILayout rows on every resize would. Did you profile it? If the Unity profiler shows the time in ToArray, I'm wrong and happy to see it. My guess is the fix is drawing only the rows in view (fixed row height, GUI.Label into rects for the visible slice), which also works with the existing snapshot.

For a yes I'd want either a profiler capture showing this change removes the hang with the tear fixed (copy under the lock), or the visible-rows approach. The 404 contributing link is fixed in #39.

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