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

correctly unmarshal elements with a type tag #53

Merged
merged 1 commit into from
Dec 28, 2023
Merged

correctly unmarshal elements with a type tag #53

merged 1 commit into from
Dec 28, 2023

Conversation

paulmach
Copy link
Owner

as described in #51 the "quick hack" to find the type does not work if the the element has "tags":{"type":"route"} like relations have.

This PR finds the tag using an unmarshal, it does impact performance.

benchmark                            old ns/op     new ns/op     delta
BenchmarkChange_UnmarshalJSON-10     1099707       1331954       +21.12%

benchmark                            old allocs     new allocs     delta
BenchmarkChange_UnmarshalJSON-10     4880           5327           +9.16%

benchmark                            old bytes     new bytes     delta
BenchmarkChange_UnmarshalJSON-10     273049        252964        -7.36%

I was able to get it down to +7% speed but much more memory using json.Decoder.Token. I opted for the simpler approach as it allowed the plugging in of "customer unmarshallers".

If performance is a concern consider using jsoniter or similar like

import (
  jsoniter "github.com/json-iterator/go"
  "github.com/paulmach/osm"
)

var c = jsoniter.Config{
  EscapeHTML:              true,
  SortMapKeys:             false,
  ValidateJsonRawMessage:  false,
  MarshalFloatWith6Digits: true,
}.Froze()

osm.CustomJSONMarshaler = c
osm.CustomJSONUnmarshaler = c

@paulmach paulmach merged commit b3aa927 into master Dec 28, 2023
3 checks passed
@paulmach paulmach deleted the type-tags branch December 28, 2023 00:26
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.

1 participant