Skip to content
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

ProcessTiming and ItemGeometry from ClientId(0) will be skipped in TuiMonitor.item_geometry #1771

Closed
Kherrisan opened this issue Jan 3, 2024 · 8 comments · Fixed by #2001
Labels
bug Something isn't working

Comments

@Kherrisan
Copy link

Kherrisan commented Jan 3, 2024

The trait Monitor will store ClientStats of each fuzzer client in a vector, and the index is the client id ClientId. So when SimpleEventManager creates ClientStats with client id 0 in handle_in_broker

monitor
    .client_stats_mut_for(ClientId(0))
    .update_user_stats(name.clone(), value.clone());

The new ClientStats will be stored in the 0-th position of the vector. Unfortunately, this position will be skipped by process_timing and item_geometry.

fn process_timing(&mut self) -> ProcessTiming {
        let mut total_process_timing = ProcessTiming::new();
        total_process_timing.exec_speed = self.execs_per_sec_pretty();
        if self.client_stats.len() > 1 {
            ......
            for client in self.client_stats().iter().skip(1) {
                new_path_time = client.last_corpus_time.max(new_path_time);
                new_objectives_time = client.last_objective_time.max(new_objectives_time);
            }
            ......
        }
        ......
    }
fn item_geometry(&self) -> ItemGeometry {
        let mut total_item_geometry = ItemGeometry::new();
        if self.client_stats.len() < 2 {
            return total_item_geometry;
        }
        ......
        for client in self.client_stats().iter().skip(1) {
        ......
    }

The result is process_timing and item_geometry is all zero in TUI.

@Kherrisan Kherrisan added the bug Something isn't working label Jan 3, 2024
@tokatoka
Copy link
Member

tokatoka commented Jan 3, 2024

@ToSeven
can you look at this?

@ToSeven
Copy link
Contributor

ToSeven commented Jan 4, 2024

Ok, let me take a look

@ToSeven
Copy link
Contributor

ToSeven commented Jan 9, 2024

The overall section of the TUI collects the statistics of all clients. When self.client_stats.len() equals 1, it means the system has two clients: one broker and one client. Therefore, only one client has statistics.

@tokatoka
Copy link
Member

tokatoka commented Jan 9, 2024

so can I close it?

@domenukk
Copy link
Member

domenukk commented Jan 9, 2024

The point is that SimpleEventManager doesn't have a broker, so client 0 is the important one (the only one with data).
Only LLMP Managers have a broker (and for those this behaviour is probably fine?)
We need to remove special handling for non-LLMP things (somehow)

@tokatoka
Copy link
Member

tokatoka commented Apr 4, 2024

@ToSeven can you fix this?

@domenukk
Copy link
Member

domenukk commented Apr 4, 2024

Maybe each manager should have a fixed array of ignored ids? It'll be empty for SimpleEventManager and 0 for the other guys?

@domenukk
Copy link
Member

domenukk commented Apr 4, 2024

Tried a fix in #2001

@tokatoka tokatoka closed this as completed Apr 5, 2024
domenukk added a commit that referenced this issue Apr 5, 2024
* Add option to enabled/disable client stats and fix #1771

* more fix

* fix map_density

* even more fix

* remove need for vec in Aggregator::aggregate

* fix json weirdness - remove individual clients (is that all right? )

* Make pretty
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants