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

Why bytedance/sonic? #3653

Closed
alecholmez opened this issue Jun 27, 2023 · 11 comments
Closed

Why bytedance/sonic? #3653

alecholmez opened this issue Jun 27, 2023 · 11 comments

Comments

@alecholmez
Copy link

alecholmez commented Jun 27, 2023

Description

What exactly did this library gain from including this library? #3184
From my quick look at it, it requires CGO? goccy/go-json is already really fast and a pure go implementation. It seems like there was no real value add proposal for this libraries integration and it was just ushered in.

Note:
https://github.com/bytedance/sonic#compatibility

Sonic DOES NOT ensure to support all environments, due to the difficulty of developing high-performance codes. For
developers who use sonic to build their applications in different environments (ex: developing on M1 Mac but running on linux server), or those who want to handle JSON strictly consistent with encoding/json, we provide some compatible
APIs as sonic.API

Does this means that gin web severs are no longer ARM compatible? Seems a bit odd for a web framework. Folks generating code with oapi-codegen automatically inherit this dependency regardless of using gin or echo. Seems like unwanted behavior to me.

@arp242
Copy link

arp242 commented Jun 30, 2023

Does this means that gin web severs are no longer ARM compatible?

No; it's only used if you add the sonic and avx build tags, and then only on supported platforms.

That said, a JIT in gin (even if disabled by default) seems rather unexpected to me. It's in a stable release so can't really be changed now, but IMHO a gin.SetJSONDecoder(someInterface) or something along those lines would have been much better instead of piling on JSON libraries.

@otcirtan
Copy link

I don't trust bytedance. Did you see the size of this thing? And it generates machine code?

I'm going to stop upgrading gin, and probably move away from it.

@alkiller22
Copy link

I don't trust bytedance. Did you see the size of this thing? And it generates machine code?

I'm going to stop upgrading gin, and probably move away from it.

Agree, use github.com/cloudwego/hertz as a better solution

@q5p
Copy link

q5p commented Oct 26, 2023

I believe that neither the question nor the replies are constructive, I think closing this up is a good idea.

@PureWhiteWu
Copy link

PureWhiteWu commented Oct 26, 2023

@otcirtan Bytedance also contributes to the linux kernel. Please move away from linux. DON'T use anything bytedance contributed to.

@jizhuozhi
Copy link

I don't trust bytedance. Did you see the size of this thing? And it generates machine code?

I'm going to stop upgrading gin, and probably move away from it.

Personal subjective emotions (more strictly speaking, discrimination) should never be brought into the discussion of technical solutions in the open source community.

@otcirtan
Copy link

otcirtan commented Oct 26, 2023

What's subjective about a huge increase of the attack surface for very modest gains and the influence of the CCCP? May I also note that the original questions have not been addressed, let alone answered satisfactorily?

discrimination

It's only discrimination in the original, literal sense: "recognition and understanding of the difference between one thing and another: discrimination between right and wrong".

@frankli0324
Copy link

frankli0324 commented Oct 26, 2023

oh great, here comes not only discrimination, but also f**king politics. I agree that we should close this issue.

the original questions have not been addressed

question: Does this means that gin web severs are no longer ARM compatible?

No; it's only used if you add the sonic and avx build tags, and then only on supported platforms. by @arp242

no real value add proposal for this libraries integration and it was just ushered in

it adds an option to use a faster json library on specific(widely used) platforms. I see that as an improvement (by myself)

oh,

influence of the CCCP

LOL

@jizhuozhi
Copy link

jizhuozhi commented Oct 26, 2023

@otcirtan So you can distrust any json library of choice and just support encoding/json (maybe you can trust Google). But if Gin uses any other json library we can say don't trust the other technologies it uses (I think CGO will bring attack surface).

What's subjective about a huge increase of the attack surface for very modest gains and the influence of the CCCP?

In your words it is very subjective to attack companies that actively participate in open source contributions. As you said, maybe you should stop using Go, and stop using Linux (as @PureWhiteWu said), because there is too much code contributed by Bytedance.

I agree that we should close this issue.

@Leo-stone-dot
Copy link

I don't trust bytedance. Did you see the size of this thing? And it generates machine code?

I'm going to stop upgrading gin, and probably move away from it.

It would be more helpful if you can provide another high-performance library.

@alecholmez
Copy link
Author

Our consumption of the oapi-codegen tool no longer bundles this dependency into the go.mod as a transitive dep. So this issue is no longer relevant regardless of the politics above. Thanks for closing @appleboy

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

No branches or pull requests

10 participants