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

The full tokio port, Sadly there is no change in performance detected. #723

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Vrtgs
Copy link

@Vrtgs Vrtgs commented Dec 29, 2024

That being said this was only benchmarked on my local windows machine with only a handful of ports open, not the whole internet

@PsypherPunk
Copy link
Collaborator

Checking this locally (Ubuntu):

  • TCP reports "No change in performance detected".
  • UDP, however, reports "Performance has regressed", being orders of magnitude slower.

However, I'm wondering if that could be an issue in the changes to benches/benchmark_portscan.rs rather than the code itself.

Unless it's something in the generated UDP code…

@PsypherPunk
Copy link
Collaborator

@Vrtgs, the benchmarks for UDP do seem to be significantly slower (the previous run was from `master):

Benchmarking portscan udp/portscan udp: Warming up for 3.0000 s
Warning: Unable to complete 25 samples in 20.0s. You may wish to increase target time to 29.2s, or reduce sample count to 10.
portscan udp/portscan udp
                        time:   [1.1627 s 1.1656 s 1.1684 s]
                        change: [+2306.4% +2331.4% +2356.3%] (p = 0.00 < 0.05)
                        Performance has regressed.

However, when running the tool directly (i.e. not through tests/benchmarks), I'm seeing this (for both TCP and UDP scans):

❯ cargo run --release -- --addresses 127.0.0.1 --accessible --scripts none
   Compiling rustscan v2.3.0 (/home/psypherpunk/git/RustScan)
    Finished `release` profile [optimized] target(s) in 42.78s
     Running `target/release/rustscan --addresses 127.0.0.1 --accessible --scripts none`
thread 'main' panicked at /home/psypherpunk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-par-stream-0.1.1/src/futures_unordered.rs:37:21:
there is no reactor running, must be called from the context of a Tokio 1.x runtime
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Aborted (core dumped)

@Vrtgs
Copy link
Author

Vrtgs commented Jan 11, 2025

fixed, the issue was, for some reason this project uses both async-std executor as well as futures executor, so when I purged just async-std that wasn't enough

@PsypherPunk
Copy link
Collaborator

Thanks, @Vrtgs, everything's running correctly now.

However, something's still wrong with UDP. Both benchmarks and general use are significantly slower compared to master (3 seconds vs. 3 minutes). I think it's hitting the timeout for every request, i.e. messing with the Duration::from_millis value for scanner_udp dictates the duration.

The changes to src/generated.rs are also a little odd: if I compile this locally, that file gets recreated identically to the file on master, save for the Lazy/LazyLock references, which I'd expect. Your PR, however, contains several other changes. This doesn't explain the UDP slowness, however, as they occur regardless.

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.

2 participants