Skip to content

[WIP] Cache runes in chars mode #252

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

triallax
Copy link
Contributor

@triallax triallax commented Jul 28, 2025

WIP implementation of my proposal in #35 (comment)

unsophisticated and needs further testing and benchmarking, but already quite promising: running gron.awk on a big JSON file in chars mode goes from taking 12(!) minutes to 4 seconds on my laptop with this branch in its current form.

@triallax
Copy link
Contributor Author

using caching for the other functions (e.g. index) is a bit more complicated, because they require knowing which bytes in the string map to which rune. for example, to use the cached runes in e.g. index("مرحبا", "ر"), we need to use strings.Index which takes in a string and not a slice of runes, and then need to somehow map from the "byte index" to the rune/character index

an alternative in this case is to convert the needle to runes and search for it in the haystack slice of runes, but i'm not sure if that has subtle edge cases we may care about (e.g. string normalisation? does strings.Index even handle that? do we care or even want this in the first place?)

on the other hand even with just implementing this optimisation for length and substr we're already landing decent improvements, so maybe the status quo is fine

thoughts?

@triallax
Copy link
Contributor Author

@benhoyt gentle bump, no rush but i'd love to know how you think this looks for a prototype

@benhoyt
Copy link
Owner

benhoyt commented Aug 15, 2025

Hi @triallax, I'm not opposed to the approach itself. However, I guess the main thing I'm worried about is that it significantly slows down normal (bytes-mode) performance, due I suppose to all the additional allocations from the new([]rune) when creating each new value. I did a test locally (with simple.awk from https://github.com/benhoyt/countwords and kjvbible_x10.txt) and in bytes mode it was about twice as slow, or took twice as long, with this change.

We could try to have a switch in the interpreter which avoids the allocation if in bytes mode. The wiring for that might be annoying, but at least we could test performance. Even then I'd want to make sure the if/else in the hot path didn't slow bytes-mode down much -- GoAWK's been fairly focussed on performance, and I'd hate to regress much.

In any case, it'd be good if you could show some performance numbers, and we could come up with a way to avoid the extra new() / allocation for every string value.

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