Skip to content

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Mar 1, 2021

encoding/json is slow.

jsoniter is faster.

jsoniter also has a streaming interface which we can improve other parts e.g. routers/api/v1/repo/file.go

Signed-off-by: Andrew Thornton art27@cantab.net

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added the performance/speed performance issues with slow downs label Mar 1, 2021
@6543
Copy link
Member

6543 commented Mar 1, 2021

for build/generate-emoji.go it's fine - but for the gitea binary can we init jsoniter.ConfigCompatibleWithStandardLibrary once?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 1, 2021
@zeripath
Copy link
Contributor Author

zeripath commented Mar 1, 2021

that's not initing - it's just a reference

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 1, 2021
@techknowlogick techknowlogick added this to the 1.14.0 milestone Mar 1, 2021
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 1, 2021
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Copy link
Contributor Author

zeripath commented Mar 1, 2021

This also needs chi to migrate to jsoniter too.

@zeripath
Copy link
Contributor Author

zeripath commented Mar 1, 2021

https://gitea.com/go-chi/binding/pulls/1

@6543
Copy link
Member

6543 commented Mar 1, 2021

@zeripath I think you can update our binding middleware here ...

... done

@6543 6543 merged commit f0e1525 into go-gitea:master Mar 1, 2021
@zeripath zeripath deleted the jsoniter branch March 1, 2021 21:12
@silverwind
Copy link
Member

silverwind commented Mar 4, 2021

json-iterator/go is not always compatible with encoding/json, see their issue tracker for examples. I personally have given up on it already.

goccy/go-json may be a better alternative, it claims to be faster than json-iterator/go, but it's probably not as well tested as encoding/json.

@6543
Copy link
Member

6543 commented Mar 4, 2021

we need benchmarks for gitea!!!

@zeripath
Copy link
Contributor Author

zeripath commented Mar 4, 2021

@silverwind we only need to care about the incompatibilities that actually matter - if we're not hitting anything right now it's easy enough to handle in the future.

Not wedded to json-iterator though - looking at the code it certainly could be a lot faster.

My main reason was that it had a streaming interface but its API is not great.

Happy to try go-json if you think that's better.

@6543
Copy link
Member

6543 commented Mar 4, 2021

iterator is is opensource why not improve it upstream ... & this are now topics witch we should talk about in discord ... locking this issue now :D

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Mar 4, 2021
@zeripath
Copy link
Contributor Author

zeripath commented Mar 4, 2021

we need benchmarks for gitea!!!

So in regards to JSON libraries our testsuite as a whole is actually a rather good benchmark.

It would be useful though to have a benchmark that just pushes files to a repo over the API as I know that that is something we're surprisingly slow at.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. performance/speed performance issues with slow downs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants