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

Testbed: 32 bit memory layout for buffer #1854

Closed
wants to merge 22 commits into from

Conversation

jerch
Copy link
Member

@jerch jerch commented Dec 21, 2018

Testbed PR to be able to identify differences between 32 and 16 bit memory layout for the buffer in runtime and memory usage.

Implements:

  • 32 bit buffer layout as discussed in Buffer performance improvements #791
  • utf8 input encoding
  • direct buffer access in InputHandler.print (therefore the JS array buffer will not work here)

Buffer layout:

|             uint32_t             |        uint32_t         |        uint32_t         |
|             `content`            |          `FG`           |          `BG`           |
| wcwidth(2) comb(1) codepoint(21) | flags(8) R(8) G(8) B(8) | flags(8) R(8) G(8) B(8) |

Else its based on current master, additionally it includes the webgl renderer as default (makes it easier to spot buffer related things in the timeline).
Simply run the demo app and compare the timeline results with the 16 bit variant #1855.

Note: To see the impact of the utf8 input handling alone, the timeline results can be compared to the webgl PR branch.

/cc @Tyriar, @mofux, @bgw

Edit: The tests will not pass, I did not apply the change all over the codebase, just to the critical parts. If the build does not finish, run npm run watch and npm run watch-addons instead.

@jerch
Copy link
Member Author

jerch commented Dec 21, 2018

First results (tested with ls -lR /usr/lib in demo app with default settings):

grafik

grafik

grafik

grafik

@jerch
Copy link
Member Author

jerch commented Dec 22, 2018

Results in xterm-benchmark:

            Case "#1" : 10 runs - average runtime: 1545.61 ms
            Case "#1" : 10 runs - average throughput: 31.27 MB/s

@Tyriar
Copy link
Member

Tyriar commented Dec 27, 2018

I think this is definitely the way to go to start with, double the (small amount of) memory for a little faster runtime and it's way more simple especially since we don't have to deal with how attributes are stored and invalidated.

@Tyriar
Copy link
Member

Tyriar commented Dec 27, 2018

@jerch let's move the memory layout discussion over to #484, I think we're in agreement for how the buffer should work initially and it's sounding like we can probably get true color going in 3.11 if we go with this lower risk approach?

@jerch
Copy link
Member Author

jerch commented Dec 28, 2018

@Tyriar Yes, with this approach here the extended attrs are much easier and less error prone to implement, prolly can be done in one true color PR.

@jerch
Copy link
Member Author

jerch commented Dec 29, 2018

Closing the PR as I think we all agree to go with this approach for now.

@jerch jerch closed this Dec 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-in-progress Do not merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants