jtdaugherty / vty

A high-level ncurses alternative written in Haskell
BSD 3-Clause "New" or "Revised" License
319 stars 57 forks source link

perf: Use `ByteString` instead of `[Char]` for input parsing. #233

Closed iphydf closed 2 years ago

iphydf commented 2 years ago

NOTE: This breaks lots of internal APIs, many of which are actually exposed as public API. brick does not break, but client code that depends on constants containing escape sequences will need to adapt.

Related to https://github.com/jtdaugherty/vty/issues/231. Doesn't fix it completely, but improves the situation.

There's really no point in using [Char] here. The first thing the code did was convert all the bytes individually to Char and putting them into a linked list. Then, when actually parsing utf-8, it turns those Chars back into Word8s. All the other code is generally concerned with bytes and goes out of its way to deal with [Char] instead. The code is now simpler and much faster.

Overall this gives another order of magnitude speedup over the previous speedup totalling roughly a 200x speedup over 2 commits ago for the 300KB case (it's less for smaller cases and much more for larger cases, because we already made one n^2 algorithm n).

This does not fix the polynomial time complexity, but at this point we can comfortably paste a low number of megabytes into the terminal and process it reasonably quickly. This is sufficient to support small file uploads via bracketed paste.

image
time/size = 7.025E-11KB^3 + 4.424E-7KB^2 + 1.387E-3KB - 0.104

Some timing:

iphydf commented 2 years ago

Please review carefully. I've tested it, but not super exhaustively (my application doesn't have mouse or other things). I haven't reviewed it myself yet, so I'll do that now as well.

jtdaugherty commented 2 years ago

Have you yet attempted a build with --enable-tests?

iphydf commented 2 years ago

Have you yet attempted a build with --enable-tests?

No, I'll try that now.

iphydf commented 2 years ago

All test pass.

jtdaugherty commented 2 years ago

Okay, great. Let me know once you've finished your own review and I'll take a closer look (probably later today, or in the next few days at the latest).

iphydf commented 2 years ago

I've finished my own review :).

jtdaugherty commented 2 years ago

This change is now released in 5.35.