Add package dynamic. #599
Merged
Add package dynamic. #599
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
echlebek
requested review from
amdprophet,
cwjohnston,
portertech,
jamesdphillips,
grepory,
mercul3s,
palourde and
nikkictl
Codecov Report
@@ Coverage Diff @@
## master #599 +/- ##
==========================================
- Coverage 56.44% 56.18% -0.27%
==========================================
Files 331 332 +1
Lines 21971 22137 +166
Branches 50 50
==========================================
+ Hits 12402 12437 +35
- Misses 7668 7823 +155
+ Partials 1901 1877 -24
Continue to review full report at Codecov.
|
echlebek
force-pushed
the
proposal/package-dynamic
branch
from
8a23f5b
to
b808c8b
Compare
|
We will eventually be serializing to protobuf in etcd, can we adapt this package to also work with protobuf's any? |
|
@grepory the intent is that extended attributes will remain JSON-encoded regardless of how we marshal the envelope around the extended attributes. This will make sure we preserve the semantics of what the user gave us. So the protobuf field for extended attributes would be |
|
Oh right! Thanks for the clarification. |
grepory
approved these changes
Nov 23, 2017
Package dynamic can be used to with json.Marshaler and json.Unmarshaler to create types that can support arbitrary custom attributes. The package achieves this by parsing json messages with the json-iterator package in a streaming fashion, and mapping the results onto dynamically created types with package reflect. Package dynamic also implements GetField, which can be used to implement the govaluate.Parameters interface.
Add tests for json Marshal/Unmarshal. Refs #586
Refs #586
Refs #586 goos: linux goarch: amd64 pkg: github.com/sensu/sensu-go/types/dynamic BenchmarkQueryGovaluateSimple-4 500000 2069 ns/op 992 B/op 25 allocs/op BenchmarkQueryGovaluateComplex-4 200000 6212 ns/op 2544 B/op 75 allocs/op BenchmarkUnmarshal-4 200000 6492 ns/op 6568 B/op 57 allocs/op BenchmarkMarshal-4 300000 4556 ns/op 6512 B/op 40 allocs/op PASS ok github.com/sensu/sensu-go/types/dynamic 6.172s
This change makes the package significantly easier to use, and also presents a greater symmetry between its types and functions. This comes at a cost of additional reflection, which can be seen in the benchmark for Unmarshal. Old: BenchmarkUnmarshal-4 6492 ns/op 6568 B/op 57 allocs/op New: BenchmarkUnmarshal-4 9244 ns/op 7592 B/op 77 allocs/op Refs #586
Now that govaluate has been modified to recursively query
Parameters, we can return a lazy tree of AnyParameters instead of
dynamically generating structs. This saves quite a bit of code,
CPU time, and memory allocations.
Old struct generation
BenchmarkQueryGovaluateSimple-4 2069 ns/op 992 B/op 25 allocs/op
BenchmarkQueryGovaluateComplex-4 6212 ns/op 2544 B/op 75 allocs/op
New lazy json parsing
BenchmarkQueryGovaluateSimple-4 1404 ns/op 320 B/op 18 allocs/op
BenchmarkQueryGovaluateComplex-4 2617 ns/op 512 B/op 30 allocs/op
Refs #586
Reduces cost of querying in the most common case. Refs #586
* Break up Attributer interface into AttrGetter and AttrSetter. * Get rid of the Attributes type in favour of simple []byte. * Make AttrGetter method set match what protobuf might generate. * When returning struct fields from GetField, use reflect.Indirect so that pointers are never returned. * Make sure that extended attributes are never returned by GetField and never marshaled by Marshal. Refs #586
echlebek
force-pushed
the
proposal/package-dynamic
branch
from
2845cdf
to
6a3137d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this change?
Package dynamic can be used to with json.Marshaler and
json.Unmarshaler to create types that can support arbitrary
custom attributes.
The package achieves this by parsing json messages with
the json-iterator package in a streaming fashion, and mapping the
results onto dynamically created types with package reflect.
Package dynamic also implements GetField, which can be used to
implement the govaluate.Parameters interface.
Why is this change necessary?
Fixes #586
Were there any complications while making this change?
There were several. As part of this change, I've filed this PR against govaluate: Knetic/govaluate#84