Measure cache memory usage

[jdm]

After #18676 merges we'll want to add support for measuring how much memory is in use by cached responses.

[jdm]

Required work:

• make the memory cache data structure derive MallocSizeOf
• add a new IpcReceiver argument to ResourceChannelManager::start that is used for listening to messages from the memory profiler (it must correspond with this sender)
• use run_with_memory_reporting in the network code to register a memory reporter for that thread
• when a message from the memory profiler arrives, create a report that includes that size of the public and private http caches

[jdm]

Run Servo with ./mach run --memory-profile 5 to print a memory profile every 5 seconds. If these changes are successful, there should be a new entry for the network cache.

[olmanz]

I would want to try this.

[jdm]

Great! Please ask questions if anything is unclear.

[olmanz]

After changing

/// A memory cache.
pub struct HttpCache {
    /// cached responses.
    entries: HashMap<CacheKey, Vec<CachedResource>>,
}

to

/// A memory cache.
#[derive(MallocSizeOf)]
pub struct HttpCache {
    /// cached responses.
    entries: HashMap<CacheKey, Vec<CachedResource>>,
}

I get this error message during the build:

   Compiling net v0.0.1 (file:///home/olmanz/Schreibtisch/workspaces/Rust/servo/components/net)
error: cannot find derive macro `MallocSizeOf` in this scope
  --> components/net/http_cache.rs:93:10
   |
93 | #[derive(MallocSizeOf)]
   |          ^^^^^^^^^^^^

error: aborting due to previous error

error: Could not compile `net`.

To learn more, run the command again with --verbose.
Build FAILED in 0:00:08

I already found out that there is a MallocSizeOf in the crate malloc_size_of. But when I import it with

#[macro_use] extern crate malloc_size_of;
use malloc_size_of::MallocSizeOf;

it says

   Compiling net v0.0.1 (file:///home/olmanz/Schreibtisch/workspaces/Rust/servo/components/net)
error[E0463]: can't find crate for `mallo_size_of`
  --> components/net/http_cache.rs:10:14
   |
10 | #[macro_use] extern crate mallo_size_of;
   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate

error: aborting due to previous error

error: Could not compile `net`.

To learn more, run the command again with --verbose.
Build FAILED in 0:00:08

What am I missing here?

[jdm]

For one thing, that error message says mallo_size_of. However, be sure to add the crate to the Cargo.toml, as well.

[olmanz]

add a new IpcReceiver argument to ResourceChannelManager::start that is used for listening to messages from the memory profiler (it must correspond with this sender)

The sender is a IpcSender<ReporterRequest>, so the new argument must be of type IpcReceiver<ReporterRequest> if I understand correctly?

use run_with_memory_reporting in the network code to register a memory reporter for that thread

I see that run_with_memory_reporting must be called on profiler_chan here, but the function also takes some arguments (reporter_name, channel_for_reporter and msg), and I currently don't know where to get them inside the thread.

[olmanz]

@jdm
I'm still struggling with that part:
use run_with_memory_reporting in the network code to register a memory reporter for that thread
I searched for other instances where run_with_memory_reporting is used, e.g. in components/script/script_thread.rs and components/layout_thread/lib.rs.
As I understand it, I have to call the function on profiler_chan in components/net/resource_thread.rs.
But I'm not quite sure about the where to get the parameters that function should take. Can you give me a tip?

[jdm]

The other parameters are:

• the closure that will be executed - this is just the code that already exists, but turned into a closure
• the reporter name - something like "network-cache-reporter"
• a channel that will be used by the profiler thread to communicate with this code - I recommend creating a new channel, making ResourceChannelManager::start listen to the receiver, and passing the sender as this argument
• a constructor for the message that will be sent on the channel - if the channel is only used for reporting, it should be enough to pass || () (which will provide a closure that returns a () value, meaning that the channel will only send and receive () values)

[olmanz]

@jdm
I changed the code block

thread::Builder::new().name("ResourceManager".to_owned()).spawn(move || {
        let resource_manager = CoreResourceManager::new(
            user_agent, devtools_chan, profiler_chan
        );

        let mut channel_manager = ResourceChannelManager {
            resource_manager: resource_manager,
            config_dir: config_dir,
        };
        channel_manager.start(public_setup_port,
                              private_setup_port);
}).expect("Thread spawning failed");

to this:

thread::Builder::new().name("ResourceManager".to_owned()).spawn(move || {
        let resource_manager = CoreResourceManager::new(
            user_agent, devtools_chan, time_profiler_chan
        );

        let mut channel_manager = ResourceChannelManager {
            resource_manager: resource_manager,
            config_dir: config_dir,
        };

        let reporter_name = String::from("network-cache-reporter");
        let chan = channel();
        mem_profiler_chan.run_with_memory_reporting(|| {
            channel_manager.start(public_setup_port,
                                  private_setup_port);
        }, reporter_name, chan.0, || ());

    }).expect("Thread spawning failed");

I'm passing || () now as recommended, but I get this error message now:

   Compiling net v0.0.1 (file:///home/olmanz/Schreibtisch/workspaces/Rust/servo/components/net)
error[E0593]: closure is expected to take 1 argument, but it takes 0 arguments
  --> components/net/resource_thread.rs:88:27
   |
88 |         mem_profiler_chan.run_with_memory_reporting(|| {
   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^ expected closure that takes 1 argument
...
91 |         }, reporter_name, chan.0, || ());
   |                                   -- takes 0 arguments

error: aborting due to previous error

error: Could not compile `net`.

To learn more, run the command again with --verbose.
Build FAILED in 0:00:10

Do you have a suggestion what I can do to fix this error?

[jdm]

Oh, you can use |_| () to pass a closure that ignores the argument.

[olmanz]

Thanks @jdm, that worked perfectly.

I still got some questions though (sorry for that, I'm still learning ;-)).

You said in your initial post

add a new IpcReceiver argument to ResourceChannelManager::start that is used for listening to messages from the memory profiler (it must correspond with this sender)

since the sender is of the type IpcSender<ReporterRequest>, I guess the receiver must be of the type IpcReceiver<ReporterRequest>, and that this receiver will then be a third parameter in ResourceChannelManager::start. As of now, that function only has two parameters which are both of the type IpcReceiver<CoreResourceMsg>. How exactly can I use this new argument to listen to message from the memory profiler?

Also, ResourceChannelManager::start is now called inside the closure argument of run_with_memory_reporting(). I must pass the new receiver argument to ResourceChannelManager::start here. I tried to create a new channel with ipc::channel()::unwrap() and pass the receiver to ResourceChannelManager::start while passing the sender as the third argument in run_with_memory_reporting(), but that doesn't seem to work as the compiler delivers me the panic message the trait profile_traits::mem::OpaqueSender<>is not implemented foripc_channel::ipc::IpcSender<>``

What's the right way to create the channel and use the sender and receiver here?

[jdm]

The error message can be avoided if you implement the trait that it is complaining about.

[jdm]

Oh, and as for how to use it - add the new receiver to the receiver set, and check for the receiver's id along with the existing ones inside the loop in ResourceChannelManager::start.

[avadacatavra]

Hi @olmanz how is this going? Is there anything we can help with?

[olmanz]

@avadacatavra
Still working on it, though I was busy with some other projects at my university for the last days.
This issue is way more tricky than I thought though, so I would appreciate every help.

You can see the current status here on this branch of my fork: https://github.com/olmanz/servo/commits/issue19251

Currently, I'm getting this error message:

   Compiling net v0.0.1 (file:///home/olmanz/Schreibtisch/workspaces/Rust/servo/components/net)
   Compiling webvr v0.0.1 (file:///home/olmanz/Schreibtisch/workspaces/Rust/servo/components/webvr)
   Compiling compositing v0.0.1 (file:///home/olmanz/Schreibtisch/workspaces/Rust/servo/components/compositing)
   Compiling webdriver_server v0.0.1 (file:///home/olmanz/Schreibtisch/workspaces/Rust/servo/components/webdriver_server)
error[E0308]: mismatched types
  --> components/net/resource_thread.rs:93:39
   |
93 |         }, reporter_name, chan.0, |_| ());
   |                                       ^^ expected struct `profile_traits::mem::ReporterRequest`, found ()
   |
   = note: expected type `profile_traits::mem::ReporterRequest`
              found type `()`

error: aborting due to previous error

error: Could not compile `net`.
warning: build failed, waiting for other jobs to finish...
error: build failed
[Warning] Could not generate notification! Optional Python module 'dbus' is not installed.
Build FAILED in 0:01:10

[jdm]

Ah, my mistake. If you do |chan| chan (instead of |_| ()) and make start accept a IpcReceiver<ReportsChan> then it should work as expected.

[jdm]

@olmanz Are you still working on this?

[olmanz]

Hello @jdm,
sorry, but I couldn't do anything for the last couple of weeks because exams at my university are coming closer and closer and this is stressing me at the moment.
Exams will be over around mid-February, from that on I can surely continue to work on this issue. If this is too late for you, I don't have anything against someone else taking on this issue.

[pandusonu2]

This looks interesting :D As a beginner, can I attempt to solve this?

[jdm]

@pandusonu2 Yes, and please ask questions about anything that's unclear!

[jdm]

@pandusonu2 Are you still working on this? Have you made any progress?

[pandusonu2]

@jdm Yeah! I'm trying to work to understand as much as possible. I refered to @olmanz branch and I understood the following changes have been implemented by him

• Make HttpCache derive MallocSizeOf
  -- Include malloc_size_of and malloc_size_of_derive in Cargo.toml file
  -- Make CacheKey derive MallocSizeOf. Hence implement it for CacheKey, CacheResource, CacheResourceData
• use run_with_memory_reporting in the network code to register a memory reporter for that thread
  -- For which there is addition of mem::ProfileChan in argument list of new_resource_threads()
  -- Include this in all occurences of new_resource_threads() and new_resource_thread()
• [Current] add a new IpcReceiver argument to ResourceChannelManager::start that is used for listening to messages from the memory profiler (it must correspond with this sender)
  -- Added IpcReceiver in the argument list and also assert its id inside the loop

I'm kinda getting errors at mem_profile_chan.run_with_memory_reporting(....)
I know its easier to just ask you, but I'm new to Rust as a whole, so am trying to understand what exactly is happening 😓 So sorry that I'm taking too much time on this, if I don't get it done by this weekend, will definitely ask you 😃

[modal17]

Heyo,

Although I am not assigned, is it possible for me to look into this too?

[modal17]

@pandusonu2 Are you working on this? Is it okay if I give it an attempt? @jdm

[jdm]

@modal17 Did you delete a question that you previously asked? I swear I remember seeing one about what to measure.

[modal17]

@jdm I did! I have the memory reporter displaying messages when running with --memory-profile At the moment, I'm slowly making the memory cache derive MallocSizeOf!