Fixes #16197: Serialization of NodeConfigurations is fairly expensive #4193
Conversation
|
PR rebased |
fanf
force-pushed
the
arch_16197/serialization_of_nodeconfigurations_is_fairly_expensive
branch
from
61ae107
to
dbdbb96
Compare
|
PR updated with a new commit |
|
PR updated with a new commit |
|
PR updated with a new commit |
| * ] | ||
| */ | ||
|
|
||
| // name are false for startHour / splayHour and no override |
There was a problem hiding this comment.
i don't understand this comment
There was a problem hiding this comment.
ah, I copy/copied it. It's on the version 7 for agentrun
| def transform = JsonGlobalPolicyMode7_1(x.mode, x.overridable) | ||
| } | ||
|
|
||
| final case class JsonModes7_1( |
There was a problem hiding this comment.
could you add in comment the meaning of each of the field name. It would improve readability and avoid typos
| , vs: List[Either[List[String],JsonExpectedValueId7_1]] | ||
| ) extends JsonComponentExpectedReport7_1 { | ||
| def transform = ValueExpectedReport(vid, vs.map { | ||
| case Left(Nil) => ExpectedValueMatch("None", "None") |
There was a problem hiding this comment.
can you add in the comment that the Left is for compatibility with pre-7.1 ? I've been a bit at a lost here
There was a problem hiding this comment.
it's for compat with all techniques that don't have reportId, so more than pre-7.1, but yes I can
| ////////// json codec ////////// | ||
|
|
||
| implicit val codecJsonGlobalPolicyMode7_1: JsonCodec[JsonGlobalPolicyMode7_1] = DeriveJsonCodec.gen | ||
| implicit val codecJsonAgentRun7_0: JsonCodec[JsonAgentRun7_1] = DeriveJsonCodec.gen |
There was a problem hiding this comment.
yep. The name does not matter, but it's quite misleading :)
|
PR updated with a new commit |
|
OK, squash merging this PR |
Normation-Quality-Assistant
force-pushed
the
arch_16197/serialization_of_nodeconfigurations_is_fairly_expensive
branch
from
b0cb8e3
to
39c4ee8
Compare
https://issues.rudder.io/issues/16197
Rewrite NodeExpectedReports json serialisation with zio-json.
This is extremely boilerplaty because:
JsonNodeExpectedReportswhich is the current API with "the world outside the serialisation logic"Appart the boilerplateness, it is fairly systematic. For each version, I:
transformmethod to its correspondingJsonNodeExpectedReportscounterpart, and an implicit case class for the opposite operationAnd it's almost all.
There is a lot of code in tests, too, because well, I had to hand write JSON ADT to be sure the matching was OK.
The only a bit interesting part is the fallback logic: we try first version 7.1, then version 7.0 decoder (because we should converge with "everything in 7.1"). And we always encode in 7.1.
It's seems fast. 100k
parse(serialise)takes 19s for a 10kB json, so with my high precision measure, 0.2 ms for aparse(serialise).Lift was taking 10s for 10k, so that's at least 5x better.
The new version is takes around 50% less space. That should also help performances.