Allow to share resources, expose more debug info

[atuchin-m]

The PR changes the public API to:

• provide ability to share resources between the engines (that will save about 2 MB of memory in the browser process after it is supported in brave-core)
• expose more debug info to see the more detailed memory usage.

[github-actions]

Rust Benchmark

Benchmark suite	Current: 9294cfe	Previous: 3652b04	Ratio
rule-match-browserlike/brave-list	2250677125 ns/iter (± 15810522)	2235196924 ns/iter (± 17061743)	1.01
rule-match-first-request/brave-list	1127666 ns/iter (± 15192)	1043546 ns/iter (± 11475)	1.08
blocker_new/brave-list	148289276 ns/iter (± 999082)	157224302 ns/iter (± 336287)	0.94
blocker_new/brave-list-deserialize	21425827 ns/iter (± 796126)	63494627 ns/iter (± 260374)	0.34
memory-usage/brave-list-initial	10072555 ns/iter (± 3)	18344519 ns/iter (± 3)	0.55
memory-usage/brave-list-initial/max	64817658 ns/iter (± 3)	66961309 ns/iter (± 3)	0.97
memory-usage/brave-list-initial/alloc-count	1534723 ns/iter (± 3)	1616130 ns/iter (± 3)	0.95
memory-usage/brave-list-1000-requests	2516487 ns/iter (± 3)	2551938 ns/iter (± 3)	0.99
memory-usage/brave-list-1000-requests/alloc-count	66641 ns/iter (± 3)	68864 ns/iter (± 3)	0.97
url_cosmetic_resources/brave-list	198790 ns/iter (± 1195)	207270 ns/iter (± 3089)	0.96
cosmetic-class-id-match/brave-list	16662594 ns/iter (± 4966934)	4335864 ns/iter (± 1196471)	3.84

This comment was automatically generated by workflow using github-action-benchmark.

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.10.

Benchmark suite	Current: 9294cfe	Previous: 3652b04	Ratio
cosmetic-class-id-match/brave-list	16662594 ns/iter (± 4966934)	4335864 ns/iter (± 1196471)	3.84

This comment was automatically generated by workflow using github-action-benchmark.

[antonok-edm]

Let's implement this using traits instead; it will give us more flexibility to e.g. use an on-disk cache in brave-core and have better devtools diagnostics in the future.

More or less like this:

/// Customizable backend for [Resource] storage.
/// Custom implementations could be used to enable (for example) sharing of resources between multiple [Engine]s, an on-disk backend, or special caching behavior.
pub trait ResourceStorageBackend {
    /// Retrieves the resource with the associated name or alias
    fn get_resource(&self, resource_ident: &str) -> Option<Resource>;
}

/// Default implementation of `ResourceStorageBackend` that stores all resources in memory.
pub struct InMemoryResourceStorageBackend {
    /// Stores each resource by its canonical name
    resources: HashMap<String, Resource>,
    /// Stores mappings from aliases to their canonical resource names
    aliases: HashMap<String, String>,
}

impl ResourceStorageBackend for InMemoryResourceStorageBackend {
    fn get_resource(&self, resource_ident: &str) -> Option<Resource> {
        // ...the current `get_internal_resource` method implementation (note `&Resource` -> `Resource`)
    }
}

impl InMemoryResourceStorageBackend {
    pub fn from_resources(resources: impl IntoIterator<Item = Resource>) -> Self {
        // ...the current `ResourceStorage::from_resources` method
    }
}

// ...

/// Unified resource storage for both redirects and scriptlets.
pub struct ResourceStorage {
   backend: Box<ResourceStorageBackend>,
}

impl ResourceStorage {
    pub fn use_backend<T>(&mut self, backend: T) where T: ResourceStorageBackend + 'static {
        self.backend = Box::new(backend);
    }
    // ...replace all `self.get_internal_resource` calls with `backend.get_resource`
}

Dropping support for individual add_resource calls is ok with me. Engine can keep the existing use_resources method to create an InMemoryResourceStorageBackend with the provided resources, as well as a new method use_resources_backend.

[atuchin-m]

The particular task I'm trying to solve it sharing the impl between the engines.

It's sounds like you suggest changing the interfaces in another place.
Right now we have only one ResourceStorage impl and no plans to support another. I have no idea what to put into InMemoryResourceStorageBackend. Can you elaborate on this?

[antonok-edm]

InMemoryResourceStorageBackend is identical to the current implementation. What I propose with the ResourceStorageBackend trait is that we can use it to define a new brave-core specific implementation that supports sharing between engines and (eventually) on-disk caching. We can put the implementation of that in brave-core's adblock binding layer since we will need tighter integration with Chromium's FS operations; it doesn't need to live in adblock-rust itself.

/// To be wrapped in [Rc] for shared access across engines
struct BraveCoreResourceStorageInner {
    /// Stores each resource by its canonical name
    resources: HashMap<String, Resource>,
    /// Stores mappings from aliases to their canonical resource names
    aliases: HashMap<String, String>,
}

#[derive(Clone)]
struct BraveCoreResourceStorage {
    shared_storage: Rc<BraveCoreResourceStorageInner>,
}

impl ResourceStorageBackend for BraveCoreResourceStorage {
    fn get_resource(&self, resource_ident: &str) -> Option<Resource> {
        // ...use `self.shared_storage.resources` and `self.shared_storage.aliases`
    }
}

[atuchin-m]

The current conception assumes that the browser should just make ResourceStorage and pass it to the both engines.

trait is that we can use it to define a new brave-core specific implementation that supports sharing between engines and (eventually) on-disk caching.

I don't have such plans right now. Also, on-disk representation takes more memory because of base64: an extra work is needed to process it or change the backend code to storage. If you have a conception how to deal with it, it would be nice to see a PR.
I would say we first need to focus on 20MB+ flatbuffer caching rather that relatively small resources.

In conclusion, I don't want to make multiple get_resource.

The only thing I'm generalizing here is getting the current ResourceStorage impl.
If you really want it, we can do it via traits: ResourceStorageGetter with two impls SimpleResourceStorage and SharableResourceStorageGetter.