Store tagged filters into the flatbuffer (#481)

Author: atuchin-m

src/blocker.rs

@@ -71,14 +71,13 @@ pub struct Blocker {
     pub(crate) importants: NetworkFilterList,
     pub(crate) redirects: NetworkFilterList,
     pub(crate) removeparam: NetworkFilterList,
-    pub(crate) filters_tagged: NetworkFilterList,
     pub(crate) filters: NetworkFilterList,
     pub(crate) generic_hide: NetworkFilterList,
 
     // Enabled tags are not serialized - when deserializing, tags of the existing
     // instance (the one we are recreating lists into) are maintained
     pub(crate) tags_enabled: HashSet<String>,
-    pub(crate) tagged_filters_all: Vec<NetworkFilter>,
+    pub(crate) tagged_filters_all: NetworkFilterList,
 
     pub(crate) enable_optimizations: bool,
 
@@ -144,7 +143,7 @@ impl Blocker {
 
         // only check the rest of the rules if not previously matched
         let filter = if important_filter.is_none() && !matched_rule {
-            self.filters_tagged
+            self.tagged_filters_all
                 .check(request, &self.tags_enabled, &mut regex_manager)
                 .or_else(|| self.filters.check(request, &NO_TAGS, &mut regex_manager))
         } else {
@@ -459,8 +458,6 @@ impl Blocker {
             }
         }
 
-        tagged_filters_all.shrink_to_fit();
-
         Self {
             csp: NetworkFilterList::new(csp, options.enable_optimizations),
             exceptions: NetworkFilterList::new(exceptions, options.enable_optimizations),
@@ -469,12 +466,14 @@ impl Blocker {
             // Don't optimize removeparam, since it can fuse filters without respecting distinct
             // queryparam values
             removeparam: NetworkFilterList::new(removeparam, false),
-            filters_tagged: NetworkFilterList::new(Vec::new(), options.enable_optimizations),
             filters: NetworkFilterList::new(filters, options.enable_optimizations),
             generic_hide: NetworkFilterList::new(generic_hide, options.enable_optimizations),
             // Tags special case for enabling/disabling them dynamically
             tags_enabled: HashSet::new(),
-            tagged_filters_all,
+            tagged_filters_all: NetworkFilterList::new(
+                tagged_filters_all,
+                options.enable_optimizations,
+            ),
             // Options
             enable_optimizations: options.enable_optimizations,
             regex_manager: Default::default(),
@@ -508,13 +507,6 @@ impl Blocker {
 
     fn tags_with_set(&mut self, tags_enabled: HashSet<String>) {
         self.tags_enabled = tags_enabled;
-        let filters: Vec<NetworkFilter> = self
-            .tagged_filters_all
-            .iter()
-            .filter(|n| n.tag.is_some() && self.tags_enabled.contains(n.tag.as_ref().unwrap()))
-            .cloned()
-            .collect();
-        self.filters_tagged = NetworkFilterList::new(filters, self.enable_optimizations);
     }
 
     pub fn tags_enabled(&self) -> Vec<String> {

src/data_format/storage.rs

@@ -196,8 +196,6 @@ struct NetworkFilterSerializeFmt<'a> {
     tag: &'a Option<String>,
     raw_line: Option<String>,
     id: &'a crate::utils::Hash,
-    opt_domains_union: &'a Option<crate::utils::Hash>,
-    opt_not_domains_union: &'a Option<crate::utils::Hash>,
 }
 
 /// Generic over `Borrow<NetworkFilter>` because `tagged_filters_all` requires `&'a NetworkFilter`
@@ -228,8 +226,6 @@ where
             tag: &v.tag,
             raw_line: v.raw_line.as_ref().map(|raw| *raw.clone()),
             id: &v.id,
-            opt_domains_union: &v.opt_domains_union,
-            opt_not_domains_union: &v.opt_not_domains_union,
         }
     }
 }
@@ -252,17 +248,6 @@ where
     storage_list.serialize(s)
 }
 
-/// Forces a `NetworkFilter` slice to be serialized by converting to
-/// an intermediate representation that is constructed with `NetworkFilterFmt` instead.
-fn serialize_storage_network_filter_vec<S>(vec: &[NetworkFilter], s: S) -> Result<S::Ok, S::Error>
-where
-    S: serde::Serializer,
-{
-    let storage_vec: Vec<_> = vec.iter().map(NetworkFilterSerializeFmt::from).collect();
-
-    storage_vec.serialize(s)
-}
-
 /// Provides structural aggregration of referenced adblock engine data to allow for allocation-free
 /// serialization.
 #[derive(Serialize)]
@@ -276,14 +261,12 @@ pub(crate) struct SerializeFormat<'a> {
     #[serde(serialize_with = "serialize_network_filter_list")]
     redirects: &'a NetworkFilterList,
     #[serde(serialize_with = "serialize_network_filter_list")]
-    filters_tagged: &'a NetworkFilterList,
-    #[serde(serialize_with = "serialize_network_filter_list")]
     filters: &'a NetworkFilterList,
     #[serde(serialize_with = "serialize_network_filter_list")]
     generic_hide: &'a NetworkFilterList,
 
-    #[serde(serialize_with = "serialize_storage_network_filter_vec")]
-    tagged_filters_all: &'a Vec<NetworkFilter>,
+    #[serde(serialize_with = "serialize_network_filter_list")]
+    tagged_filters_all: &'a NetworkFilterList,
 
     enable_optimizations: bool,
 
@@ -320,42 +303,6 @@ impl SerializeFormat<'_> {
     }
 }
 
-/// `_bug` is no longer used, and is cleaned up from future format versions.
-#[derive(Debug, Clone, Deserialize)]
-pub(crate) struct NetworkFilterDeserializeFmt {
-    pub mask: crate::filters::network::NetworkFilterMask,
-    pub filter: crate::filters::network::FilterPart,
-    pub opt_domains: Option<Vec<crate::utils::Hash>>,
-    pub opt_not_domains: Option<Vec<crate::utils::Hash>>,
-    pub redirect: Option<String>,
-    pub hostname: Option<String>,
-    pub csp: Option<String>,
-    _bug: Option<u32>,
-    pub tag: Option<String>,
-    pub raw_line: Option<String>,
-    pub id: crate::utils::Hash,
-    pub opt_domains_union: Option<crate::utils::Hash>,
-    pub opt_not_domains_union: Option<crate::utils::Hash>,
-}
-
-impl From<NetworkFilterDeserializeFmt> for NetworkFilter {
-    fn from(v: NetworkFilterDeserializeFmt) -> Self {
-        Self {
-            mask: v.mask,
-            filter: v.filter,
-            opt_domains: v.opt_domains,
-            opt_not_domains: v.opt_not_domains,
-            modifier_option: v.redirect.or(v.csp),
-            hostname: v.hostname,
-            tag: v.tag,
-            raw_line: v.raw_line.map(Box::new),
-            id: v.id,
-            opt_domains_union: v.opt_domains_union,
-            opt_not_domains_union: v.opt_not_domains_union,
-        }
-    }
-}
-
 #[derive(Debug, Deserialize, Default)]
 pub(crate) struct NetworkFilterListDeserializeFmt {
     pub flatbuffer_memory: Vec<u8>,
@@ -379,11 +326,10 @@ pub(crate) struct DeserializeFormat {
     exceptions: NetworkFilterListDeserializeFmt,
     importants: NetworkFilterListDeserializeFmt,
     redirects: NetworkFilterListDeserializeFmt,
-    filters_tagged: NetworkFilterListDeserializeFmt,
     filters: NetworkFilterListDeserializeFmt,
     generic_hide: NetworkFilterListDeserializeFmt,
 
-    tagged_filters_all: Vec<NetworkFilterDeserializeFmt>,
+    tagged_filters_all: NetworkFilterListDeserializeFmt,
 
     enable_optimizations: bool,
 
@@ -422,7 +368,6 @@ impl<'a> From<(&'a Blocker, &'a CosmeticFilterCache)> for SerializeFormat<'a> {
             exceptions: &blocker.exceptions,
             importants: &blocker.importants,
             redirects: &blocker.redirects,
-            filters_tagged: &blocker.filters_tagged,
             filters: &blocker.filters,
             generic_hide: &blocker.generic_hide,
 
@@ -465,12 +410,11 @@ impl TryFrom<DeserializeFormat> for (Blocker, CosmeticFilterCache) {
                 importants: v.importants.try_into()?,
                 redirects: v.redirects.try_into()?,
                 removeparam: NetworkFilterList::default(),
-                filters_tagged: v.filters_tagged.try_into()?,
                 filters: v.filters.try_into()?,
                 generic_hide: v.generic_hide.try_into()?,
 
                 tags_enabled: Default::default(),
-                tagged_filters_all: v.tagged_filters_all.into_iter().map(|f| f.into()).collect(),
+                tagged_filters_all: v.tagged_filters_all.try_into()?,
 
                 enable_optimizations: v.enable_optimizations,
                 regex_manager: Default::default(),

src/filters/network.rs

@@ -385,10 +385,6 @@ pub struct NetworkFilter {
     pub raw_line: Option<Box<String>>,
 
     pub id: Hash,
-
-    // All domain option values (their hashes) OR'ed together to quickly dismiss mis-matches
-    pub opt_domains_union: Option<Hash>,
-    pub opt_not_domains_union: Option<Hash>,
 }
 
 // TODO - restrict the API so that this is always true - i.e. lazy-calculate IDs from actual data,
@@ -453,8 +449,6 @@ impl NetworkFilter {
 
         let mut opt_domains: Option<Vec<Hash>> = None;
         let mut opt_not_domains: Option<Vec<Hash>> = None;
-        let mut opt_domains_union: Option<Hash> = None;
-        let mut opt_not_domains_union: Option<Hash> = None;
 
         let mut modifier_option: Option<String> = None;
         let mut tag: Option<String> = None;
@@ -497,14 +491,10 @@ impl NetworkFilter {
 
                         if !opt_domains_array.is_empty() {
                             opt_domains_array.sort_unstable();
-                            opt_domains_union =
-                                Some(opt_domains_array.iter().fold(0, |acc, x| acc | x));
                             opt_domains = Some(opt_domains_array);
                         }
                         if !opt_not_domains_array.is_empty() {
                             opt_not_domains_array.sort_unstable();
-                            opt_not_domains_union =
-                                Some(opt_not_domains_array.iter().fold(0, |acc, x| acc | x));
                             opt_not_domains = Some(opt_not_domains_array);
                         }
                     }
@@ -822,8 +812,6 @@ impl NetworkFilter {
             },
             modifier_option,
             id: utils::fast_hash(line),
-            opt_domains_union,
-            opt_not_domains_union,
         })
     }
 

src/optimizer.rs

@@ -167,9 +167,7 @@ impl Optimization for UnionDomainGroup {
         if !domains.is_empty() {
             let mut domains = domains.into_iter().cloned().collect::<Vec<_>>();
             domains.sort_unstable();
-            let opt_domains_union = Some(domains.iter().fold(0, |acc, x| acc | x));
             filter.opt_domains = Some(domains);
-            filter.opt_domains_union = opt_domains_union;
         }
         if !not_domains.is_empty() {
             let mut domains = not_domains.into_iter().cloned().collect::<Vec<_>>();

tests/unit/blocker.rs

@@ -1145,7 +1145,6 @@ mod blocker_tests {
             blocker.tags_enabled,
             HashSet::from_iter([String::from("stuff")].into_iter())
         );
-        assert_eq!(blocker.filters_tagged.get_filter_map().total_size(), 2);
 
         request_expectations
             .into_iter()
@@ -1200,7 +1199,6 @@ mod blocker_tests {
             blocker.tags_enabled,
             HashSet::from_iter([String::from("brian"), String::from("stuff")].into_iter())
         );
-        assert_eq!(blocker.filters_tagged.get_filter_map().total_size(), 4);
 
         request_expectations
             .into_iter()
@@ -1254,13 +1252,11 @@ mod blocker_tests {
             blocker.tags_enabled,
             HashSet::from_iter([String::from("brian"), String::from("stuff")].into_iter())
         );
-        assert_eq!(blocker.filters_tagged.get_filter_map().total_size(), 4);
         blocker.disable_tags(&["stuff"]);
         assert_eq!(
             blocker.tags_enabled,
             HashSet::from_iter([String::from("brian")].into_iter())
         );
-        assert_eq!(blocker.filters_tagged.get_filter_map().total_size(), 2);
 
         request_expectations
             .into_iter()

tests/unit/engine.rs

@@ -183,7 +183,7 @@ mod tests {
     fn deserialization_generate_simple() {
         let mut engine = Engine::from_rules(["ad-banner"], Default::default());
         let data = engine.serialize().unwrap();
-        assert_eq!(hash(&data), 867372640370260034, "{}", HASH_MISSMATCH_MSG);
+        assert_eq!(hash(&data), 5723845290597955159, "{}", HASH_MISSMATCH_MSG);
         engine.deserialize(&data).unwrap();
     }
 
@@ -192,7 +192,7 @@ mod tests {
         let mut engine = Engine::from_rules(["ad-banner$tag=abc"], Default::default());
         engine.use_tags(&["abc"]);
         let data = engine.serialize().unwrap();
-        assert_eq!(hash(&data), 13055424859571526788, "{}", HASH_MISSMATCH_MSG);
+        assert_eq!(hash(&data), 9626816743810307798, "{}", HASH_MISSMATCH_MSG);
         engine.deserialize(&data).unwrap();
     }
 
@@ -217,9 +217,9 @@ mod tests {
         let data = engine.serialize().unwrap();
 
         let expected_hash = if cfg!(feature = "css-validation") {
-            11273622377918966036
+            6718506180720782170
         } else {
-            3151397840865371565
+            6839468684492187294
         };
 
         assert_eq!(hash(&data), expected_hash, "{}", HASH_MISSMATCH_MSG);