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

Delay the init() of github.com/goccy/go-json #2253

Merged
merged 2 commits into from
Dec 11, 2024
Merged

Conversation

ptodev
Copy link
Contributor

@ptodev ptodev commented Dec 10, 2024

PR Description

The github.com/goccy/go-json creates a cache in its init() function which takes about 20MB of memory. This happens even if no component uses this particular json library. There is a PR upstream to change the behaviour to a lazy initialisation, and other projects such as grafana/grafana have already been patched with a fork that does it.

I'm not 100% sure which features of which components use goccy/go-json, but I think it's these packages:

  • Splunk HEC exporter
  • Stanza
  • OTTL

Notes to the Reviewer

This is a profile of a locally ran Alloy instance prior to the change:
Screenshot 2024-12-10 at 12 11 37

This is a profile after the change:
Screenshot 2024-12-10 at 11 48 31

This is its config:

otelcol.receiver.otlp "from_k6" {
    grpc {
        endpoint = "0.0.0.0:4320"
    }

    output {
    traces  = [otelcol.exporter.loadbalancing.default.input]
    }
}

otelcol.exporter.loadbalancing "default" {
  routing_key = "traceID"
    resolver {
        static {
            hostnames = ["localhost:55690"]
        }
    }
    protocol {
        otlp {
            client {
              tls {
                insecure = true
              }
            }
        }
    }
}

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@ptodev ptodev requested a review from a team as a code owner December 10, 2024 12:41
Copy link
Contributor

@wildum wildum left a comment

Choose a reason for hiding this comment

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

Awesome find, just a nit for the changelog entry

CHANGELOG.md Show resolved Hide resolved
Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

Seems reasonable and a well trod approach.

It has been causing high memory usage
even when goccy/go-json isn't used.
@ptodev ptodev force-pushed the ptodev/delay-goccy-init branch from 93e762c to 2a7ce4c Compare December 11, 2024 11:46
@ptodev ptodev merged commit ae67020 into main Dec 11, 2024
17 of 18 checks passed
@ptodev ptodev deleted the ptodev/delay-goccy-init branch December 11, 2024 12:04
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.

4 participants