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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: introduce a LRU compiled style cache for the HTML formatter #938

Merged
merged 1 commit into from Feb 27, 2024

Conversation

alecthomas
Copy link
Owner

@alecthomas alecthomas commented Feb 27, 2024

馃悮 ~/dev/chroma $ benchcmp before.txt after.txt
benchmark                    old ns/op     new ns/op     delta
BenchmarkHTMLFormatter-8     160560        77797         -51.55%

benchmark                    old allocs     new allocs     delta
BenchmarkHTMLFormatter-8     1267           459            -63.77%

benchmark                    old bytes     new bytes     delta
BenchmarkHTMLFormatter-8     52568         25067         -52.32%

```
馃悮 ~/dev/chroma $ benchcmp before.txt after.txt                                                                                                                                                            master
benchmark                    old ns/op     new ns/op     delta
BenchmarkHTMLFormatter-8     160560        77797         -51.55%

benchmark                    old allocs     new allocs     delta
BenchmarkHTMLFormatter-8     1267           459            -63.77%

benchmark                    old bytes     new bytes     delta
BenchmarkHTMLFormatter-8     52568         25067         -52.32%
```
@alecthomas alecthomas merged commit 6dd9f26 into master Feb 27, 2024
2 checks passed
@alecthomas alecthomas deleted the aat/html-style-cache branch February 27, 2024 06:05

func (l *styleCache) get(style *chroma.Style) map[chroma.TokenType]string {
l.mu.Lock()
defer l.mu.Unlock()
Copy link

Choose a reason for hiding this comment

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

Can this be turned into a sync.RWLock? Most cases will probably only write to this cache once so it'd be a shame to introduce potential lock-contention if there are many reads.

Something like https://stackoverflow.com/a/70236841 should dodge the gotchas with finishing the read-unlock-defer before grabbing the write lock

Copy link
Owner Author

Choose a reason for hiding this comment

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

Possibly, send a PR through if you're interested.

Copy link
Owner Author

Choose a reason for hiding this comment

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

FWIW I started with an RWMutex but it simplifies the code being a plain Mutex.

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.

None yet

2 participants