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

String Perf traps and invalidations #45

Closed
miguelraz opened this issue Mar 18, 2022 · 6 comments
Closed

String Perf traps and invalidations #45

miguelraz opened this issue Mar 18, 2022 · 6 comments
Labels
enhancement New feature or request

Comments

@miguelraz
Copy link

I love this repo!

But there's some easy perf steps to implement without too much hassle if you read the manual

Some of the trickier ones (but hoo boy, taking like 5s to print the make_logo :/ ) can also be found here - they should help out a ton for many of the use cases that you are using.

@FedeClaudi
Copy link
Owner

Thanks!

There's lots of room for optimization 😬

The goal so far was to get together a working, minimal, features set for people to start using Term.
Now I was planning to spend some time cleaning up the code and improving performance.

I'll have a look at the links you've sent, but if you have any advice on profiling/optimization practices that would help a lot, it's not something I've spent a lot of time doing in Julia yet.

@FedeClaudi FedeClaudi added this to To do in planning Mar 18, 2022
@FedeClaudi
Copy link
Owner

I'm surprised it took so long though, print(make_logo()) should be pretty fast:
image

@FedeClaudi FedeClaudi added the enhancement New feature or request label Mar 21, 2022
@FedeClaudi FedeClaudi moved this from To do to In progress in planning Mar 22, 2022
@FedeClaudi FedeClaudi moved this from In progress to To be tested in planning Mar 26, 2022
@FedeClaudi FedeClaudi moved this from To be tested to Done in planning Mar 26, 2022
@FedeClaudi
Copy link
Owner

#60 introduces substantial changes that reduce runtime and allocations. Pre-compilation still takes a few seconds. I'm happy to make changes if you have suggestions for that.

@miguelraz
Copy link
Author

This is awesome work to see.

I don't want to backseat drive your package's design, but here's a sketch of ideas that I didn't get a chance to test:

  1. judicious use of macros. There's defnitely some computations/formatting that can happen at parse time and not run time, and splitting those up can potentially yield some speedups. However, my gut says (last I checked) there's too many macros being used where plain functions could be used instead.
  2. Internal representation of sigils. Having stuff like [green] Hello world! [/green] represented in test itself, and then having to manipulate strings to denote the mapping "Hello world!" => [green] can lead to an increased number of string operations where none need happen. My plan for this would be to keep a vector with extra bookkeeping on where the control code combinations should be emitted but leave the original string untouched. You then iterate in tandem on the original string and the bookkeeping vector/structure, and where the control codes need to be emitted that prints out to the proper IO buffer.

Huge fan and I can't wait to see what comes next.

@FedeClaudi
Copy link
Owner

I don't want to backseat drive

Not at all, this is very useful.

  1. macros are used very sparingly in Term. Term exposes a bunch of styling macros for quick use, but I expect that most users will be using the [style] markup syntax [/style] instead.

... there's too many macros being used where plain functions could be used instead

Have you got any specific spot in place?

  1. I think I understand what you mean. Instead of "injecting" the style codes in the string, keep track of where they should appear when you stream stuff to the IO buffer. That's very clever.

After your initial issue I've spent a bunch time cleaning up the code and now the style parsing is more efficient:

pts = """Lorem[red] ipsum dolor s[/red]it amet, consectetur adipiscing elit,
ed do eiusmod tempor [bold blue]incididu[underline]nt ut labore et dolore magna aliqua. Ut enim ad minim
 veniam, quis nos[/underline]trud exercitation ullamco laboris nisi ut aliquip ex 
 ea commodo consequat. Duis aute [/bold blue][red on_black]irure dolor in reprehenderit 
 in voluptate velit esse cillum dolore eu fugiat nulla 
 pariatur. Excepteur sint occaecat[/red on_black] cupidatat non proident, 
 sunt in [green]culpa qui officia[/green] deserunt mollit anim 
 id est laborum."""

@time apply_style(pts);  # 0.000245 seconds (412 allocations: 37.781 KiB)

200ns is not great, surely what you suggest would be faster. The problem is that as you hint to, I need to cut and stitch together the whole string a bunch of times and that takes allocations. I get screwed over by the fact that strings are immutable in Julia. I've considered working with vectors of Chars instead, but not sure if that would make it any better.

You then iterate in tandem on the original string and the bookkeeping vector/structure, and where the control codes need to be emitted that prints out to the proper IO buffer.

The challenge with this approach is that you still need to remove the markup style tags []..[/] from the string, so you still end up doing a bunch of strings cutting. Also that means that once you've cut out one tag the position of the following codes changes... it gets messy.


I guess one thing worth mentioning is that I don't see Term processing huge amounts of text. It's to produce nice output for humans and there's only so much reading a human can do. The styling part is also one of the most expensive parts. So it's mostly about sparingly producing nice terminal output in you code and it would likely add a few milliseconds to runtime. It's still worth improving performance, especially when dealing with things like progressbars that are refreshed at 60Hz, but it might not be worth obsessing over it (for now).

Huge fan and I can't wait to see what comes next.

Thanks! I'm looking forward to see what you make of it!. For the time being it will be mostly about performance improvements and bug fixes, but in the future I will start adding new features like Tables.

@FedeClaudi
Copy link
Owner

By the way I did do a lot of work to improve performance (it will come with the next release). Benchmarking print_logo went from
image

to

image

a 45x speed up and a 10x reduction in allocations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants