You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "toddtreece (via GitHub)" <gi...@apache.org> on 2024/11/22 21:20:09 UTC

[PR] fix(internal/json): add arrow_json_stdlib build tag [arrow-go]

toddtreece opened a new pull request, #199:
URL: https://github.com/apache/arrow-go/pull/199

   ### Rationale for this change
   
   Grafana and Grafana plugins both use arrow-go, but do not use arrow's `internal/json`. A decent amount of used heap is initialized by https://github.com/goccy/go-json, so it would be useful to be able to prevent the initialization.
   
   The example below shows that %60 of this heap profile in a Grafana plugin is used by `go-json`'s encoder & decoder packages.
   
   ![image](https://github.com/user-attachments/assets/abd377c6-4510-4652-8801-88785f6c121d)
   
   I have submitted a [PR to go-json](https://github.com/goccy/go-json/pull/490) to switch to lazy initialization (additional details in the PR), but I am having a hard time getting a response from the maintainer, so am attempting a different approach to the problem with this PR.
   
   ### What changes are included in this PR?
   
   Adds `arrow_json_stdlib` build tag so that it's possible to switch to `encoding/json` and avoid the overhead of https://github.com/goccy/go-json.
   
   ### Are these changes tested?
   
   I tested this locally, but I can add coverage if this seems like an acceptable approach.
   
   ### Are there any user-facing changes?
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(internal/json): add arrow_json_stdlib build tag [arrow-go]

Posted by "toddtreece (via GitHub)" <gi...@apache.org>.
toddtreece commented on PR #199:
URL: https://github.com/apache/arrow-go/pull/199#issuecomment-2494887343

   @zeroshade sounds good, thanks! i will continue trying to get the `go-json` PR merged, and if i do, i'll open another PR to update the dependency here.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(internal/json): add arrow_json_stdlib build tag [arrow-go]

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #199:
URL: https://github.com/apache/arrow-go/pull/199#issuecomment-2494855264

   We mostly use `goccy/go-json` for performance reasons, but this seems to be a perfectly reasonable way to handle the situation. I definitely prefer fixing it upstream in `go-json` via your PR if possible. But this is a good stop-gap.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(internal/json): add arrow_json_stdlib build tag [arrow-go]

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #199:
URL: https://github.com/apache/arrow-go/pull/199


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(internal/json): add arrow_json_stdlib build tag [arrow-go]

Posted by "toddtreece (via GitHub)" <gi...@apache.org>.
toddtreece commented on PR #199:
URL: https://github.com/apache/arrow-go/pull/199#issuecomment-2498227061

   @zeroshade it looks like maybe you pushed a fix for the failing mac OS checks, so i rebased this against main


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org