Uh oh!
There was an error while loading. Please reload this page.
use sync.OnceValue for various regular expressions - #15
Conversation
27e92a1 to
4ca1403CompareCodecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## main #15 +/- ##
==========================================
+ Coverage 83.71% 84.81% +1.09%
==========================================
Files 5 5 Lines 393 316 -77 ==========================================
- Hits 329 268 -61 + Misses 54 38 -16
Partials 10 10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
thaJeztah
commented
Jul 15, 2024
/cc @tonistiigi |
milosgajdos
left a comment
There was a problem hiding this comment.
This is interesting. Out of curiosity. what triggered this change? Who has discovered/highlighted the memory pressure issue?
thaJeztah
commented
Jul 15, 2024
Yes; regexes being "hungry" on resources is a known issue in general, which is also why Kir made various pull requests some time ago to try to reduce their use (e.g. docker/go-units#40), but this one cam through @tonistiigi who found that For a long-lived process / daemon, it's probably less problematic, but in the CLI, this resulted in nearly 2MB to be used just by importing those packages; here's from a Slack thread on that;
So this is an attempt at reducing such cases, and making the regexes compiled on first use. |
milosgajdos
commented
Aug 5, 2024
@thaJeztah now that #16 has been merged, wanna open this for review? |
milosgajdos
commented
Nov 5, 2024
ping @thaJeztah |
| strategy: | ||
| matrix: | ||
| go-version: [1.20.x, 1.21.x] | ||
| go-version: [1.21.x, 1.22.x] |
There was a problem hiding this comment.
we should change this to 1.22.x and 1.23.x
milosgajdos
commented
Apr 11, 2025
We need to bump Go on this repo 🙈 |
thaJeztah
commented
Apr 11, 2025
@milosgajdos failure was because an invalid |
c6ab0bc to
519414fCompareUsing regex.MustCompile consumes a significant amount of memory when importing the package, even if those regular expressions are not used. This changes compiling the regular expressions to use a sync.OnceValue so that they're only compiled the first time they're used. There are various regular expressions remaining that are still compiled on import, but these are exported, so changing them to a sync.OnceValue would be a breaking change; we can still decide to do so, but leaving that for a follow-up. It's worth noting that sync.OnceValue requires go1.21 or up, so raising the minimum version accordingly. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
use sync.OnceValue for various regular expressions, require go1.21
Using regex.MustCompile consumes a significant amount of memory when
importing the package, even if those regular expressions are not used.
This changes compiling the regular expressions to use a sync.OnceValue
so that they're only compiled the first time they're used.
There are various regular expressions remaining that are still compiled
on import, but these are exported, so changing them to a sync.OnceValue
would be a breaking change; we can still decide to do so, but leaving
that for a follow-up.
It's worth noting that sync.OnceValue requires go1.21 or up, so raising
the minimum version accordingly.
Before / After (on the docker CLI (
GODEBUG=inittrace=1 ./build/docker)):