Skip to content

Decouple serialization/deserialization logic from internal data representation #102

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

Merged
merged 1 commit into from
Jun 1, 2020

Conversation

antonok-edm
Copy link
Collaborator

@antonok-edm antonok-edm commented May 28, 2020

The way rmp_serde reads the Messagepack protocol structure (it only reads as far as needed to fill the struct, even if that's in the middle of a top-level list) means that the serialized version of Blocker would not be able to gain any new fields, which is problematic for #97 (which broke forwards-compatibility).

I've abstracted out serialization and deserialization logic into a module separate from Blocker and CosmeticFilterCache. My past decision to break the CosmeticFilterCache into a completely different serialization pass makes this a little uglier, but the extra layer of separation between the data format and internal representation makes future changes in both much easier.

The extra layer of translation is also very cheap performance-wise, considering that references are used for serialization and structures built up during deserialization can be moved directly into the engine.

I've verified that backwards and forwards compatibility is maintained with DAT files from this branch and be0ba3b.

@antonok-edm antonok-edm requested a review from AndriusA May 28, 2020 04:32
@antonok-edm antonok-edm self-assigned this May 28, 2020
@antonok-edm antonok-edm merged commit a6b82b9 into master Jun 1, 2020
@antonok-edm antonok-edm deleted the structural-format branch June 1, 2020 15:56
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