perf: Reduce encoder bandwidth#430
perf: Reduce encoder bandwidth#430elementbound merged 18 commits intoepic/refactor-rollback-synchronizerfrom
Conversation
| redundancy = p_redundancy | ||
|
|
||
| func encode(tick: int) -> Array: | ||
| func encode(tick: int, properties: Array[PropertyEntry]) -> Array: |
There was a problem hiding this comment.
The argument holding Array[PropertyEntry] is called properties here, property_config in decode and in snapshot-history-encoder.gd and property_entries in diff-history-encoder.gd. Is there any reason for the discrepancy or can these names be made more consistent?
| while at < data.size(): | ||
| var property_idx := data.decode_u8(at) | ||
| var property_value := data.decode_var(at + 1) | ||
| var property_size := data.decode_var_size(at + 1) |
There was a problem hiding this comment.
I would have used a StreemPeerBuffer for en/decoding. Imho this would lead to more robust code. But this works too.
There was a problem hiding this comment.
Neat, wasn't aware of that class
|
|
||
| func put(key: Variant, value: Variant) -> void: | ||
| _keys_to_values[key] = value | ||
| _values_to_keys[value] = key |
There was a problem hiding this comment.
This breaks as soon as two values have the same key. There has to be a check to guarantee the bidirectional mapping.
There was a problem hiding this comment.
That is not the intended use. This class is for one-to-one relations as mentioned at the top of the file.
There was a problem hiding this comment.
I still would add an assert to make sure this invariant is upheld at least in debug builds.
There was a problem hiding this comment.
I still consider this as standard behavior for the data structure. Same way the dictionary doesn't warn you when you're trying to associate multiple values with the same key - it just overwrites the association. The same happens here.
Granted, I'm willing to add an assert or warning down the line in case we run into a situation where this behavior gets confusing, but I do not see the point at this time.
There was a problem hiding this comment.
If it would only overwrite the old data I would be fine with it. But in unfortunate cases we end up in an inconsistent state.
For example, we first add a mapping from a -> b, then a mapping from c -> b.
_keys_to_values is then { a -> b, c -> b} and _values_to_keys is { b -> c }
There was a problem hiding this comment.
Would have helped if you started with that example 😄 I think that was fixed a bit before you posted this comment, but let me know if the tests don't cover that case.
There was a problem hiding this comment.
Should be fixed now. Yes, I should have provided the example initially, I'm sorry.
Removes the property paths from the sent state and input data. Instead: - SnapshotHistoryEncoder: For full states, simply send an array of values - 104 bytes -> 48 bytes - DiffHistoryEncoder: For diff states, send a single byte property index and then property value - 44 bytes -> ~~28~~ 32 bytes - RedundantHistoryEncoder: Send a flattened array of input values - 248 bytes -> 80 bytes Refs #358 Closes #159
Removes the property paths from the sent state and input data. Instead: - SnapshotHistoryEncoder: For full states, simply send an array of values - 104 bytes -> 48 bytes - DiffHistoryEncoder: For diff states, send a single byte property index and then property value - 44 bytes -> ~~28~~ 32 bytes - RedundantHistoryEncoder: Send a flattened array of input values - 248 bytes -> 80 bytes Refs #358 Closes #159
Removes the property paths from the sent state and input data. Instead: - SnapshotHistoryEncoder: For full states, simply send an array of values - 104 bytes -> 48 bytes - DiffHistoryEncoder: For diff states, send a single byte property index and then property value - 44 bytes -> ~~28~~ 32 bytes - RedundantHistoryEncoder: Send a flattened array of input values - 248 bytes -> 80 bytes Refs #358 Closes #159
Removes the property paths from the sent state and input data. Instead: - SnapshotHistoryEncoder: For full states, simply send an array of values - 104 bytes -> 48 bytes - DiffHistoryEncoder: For diff states, send a single byte property index and then property value - 44 bytes -> ~~28~~ 32 bytes - RedundantHistoryEncoder: Send a flattened array of input values - 248 bytes -> 80 bytes Refs #358 Closes #159
Removes the property paths from the sent state and input data. Instead: - SnapshotHistoryEncoder: For full states, simply send an array of values - 104 bytes -> 48 bytes - DiffHistoryEncoder: For diff states, send a single byte property index and then property value - 44 bytes -> ~~28~~ 32 bytes - RedundantHistoryEncoder: Send a flattened array of input values - 248 bytes -> 80 bytes Refs #358 Closes #159
Removes the property paths from the sent state and input data. Instead:
2832 bytesRefs #358
Closes #159