From 2e9ab0625e9e58995d8376bf3d41b62bce9d2640 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Thu, 23 Jan 2020 06:23:53 +0900 Subject: [PATCH 1/9] Tweak actions (#1305) * Add benchmark action * Fix Windows build --- .github/workflows/bench.yml | 42 +++++++++++++++++++++++++++++++++++ .github/workflows/macos.yml | 2 +- .github/workflows/windows.yml | 22 +++++++++++++----- 3 files changed, 59 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/bench.yml diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml new file mode 100644 index 000000000..3c9deb49c --- /dev/null +++ b/.github/workflows/bench.yml @@ -0,0 +1,42 @@ +name: Benchmark (Linux) + +on: [push, pull_request] + +jobs: + check_benchmark: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@master + + - name: Install Rust + uses: actions-rs/toolchain@v1 + with: + toolchain: nightly + profile: minimal + override: true + + - name: Generate Cargo.lock + uses: actions-rs/cargo@v1 + with: + command: generate-lockfile + - name: Cache cargo registry + uses: actions/cache@v1 + with: + path: ~/.cargo/registry + key: ${{ matrix.version }}-x86_64-unknown-linux-gnu-registry-${{ hashFiles('**/Cargo.lock') }} + - name: Cache cargo index + uses: actions/cache@v1 + with: + path: ~/.cargo/git + key: ${{ matrix.version }}-x86_64-unknown-linux-gnu-cargo-index-${{ hashFiles('**/Cargo.lock') }} + - name: Cache cargo build + uses: actions/cache@v1 + with: + path: target + key: ${{ matrix.version }}-x86_64-unknown-linux-gnu-cargo-build-${{ hashFiles('**/Cargo.lock') }} + + - name: Check benchmark + uses: actions-rs/cargo@v1 + with: + command: bench diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index f50ae2f05..d30d6b3e9 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -27,7 +27,7 @@ jobs: - name: Generate Cargo.lock uses: actions-rs/cargo@v1 with: - command: update + command: generate-lockfile - name: Cache cargo registry uses: actions/cache@v1 with: diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 9aa3d3ba4..52c82a886 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -3,6 +3,7 @@ name: CI (Windows) on: [push, pull_request] env: + VCPKG_HASH: 3f62e5d55d1a7d8905df35d5c441d6e9ad64ffdf VCPKGRS_DYNAMIC: 1 jobs: @@ -30,7 +31,7 @@ jobs: - name: Generate Cargo.lock uses: actions-rs/cargo@v1 with: - command: update + command: generate-lockfile - name: Cache cargo registry uses: actions/cache@v1 with: @@ -50,14 +51,23 @@ jobs: uses: actions/cache@v1 id: cache-vcpkg with: - path: C:\vcpkg - key: windows_x64-${{ matrix.version }}-vcpkg + path: vcpkg + key: windows_x64-${{ env.VCPKG_HASH }}-vcpkg - name: Install OpenSSL if: steps.cache-vcpkg.outputs.cache-hit != 'true' + shell: pwsh run: | - vcpkg integrate install - vcpkg install openssl:x64-windows + git clone https://github.com/Microsoft/vcpkg.git + cd vcpkg + git reset --hard $VCPKG_HASH + .\bootstrap-vcpkg.bat + .\vcpkg integrate install + .\vcpkg install openssl:x64-windows + Copy-Item .\installed\x64-windows\bin\libcrypto-1_1-x64.dll .\installed\x64-windows\bin\libcrypto.dll + Copy-Item .\installed\x64-windows\bin\libssl-1_1-x64.dll .\installed\x64-windows\bin\libssl.dll + Get-ChildItem .\installed\x64-windows\bin + Get-ChildItem .\installed\x64-windows\lib - name: check build uses: actions-rs/cargo@v1 @@ -76,4 +86,4 @@ jobs: --skip=test_simple --skip=test_expect_continue --skip=test_http10_keepalive - --skip=test_slow_request + --skip=test_slow_request From a3287948d19bbdc9e7cf9957403961eeb2d8b94d Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Thu, 23 Jan 2020 01:08:23 +0000 Subject: [PATCH 2/9] allow explicit SameSite=None cookies (#1282) fixes #1035 --- MIGRATION.md | 6 ++++++ actix-http/CHANGES.md | 6 ++++++ actix-http/src/cookie/draft.rs | 16 ++++++++++++---- actix-http/src/cookie/mod.rs | 6 ++---- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 529f9714d..aef382a21 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1,3 +1,9 @@ +## Unreleased + +* Setting a cookie's SameSite property, explicitly, to `SameSite::None` will now + result in `SameSite=None` being sent with the response Set-Cookie header. + To create a cookie without a SameSite attribute, remove any calls setting same_site. + ## 2.0.0 * `HttpServer::start()` renamed to `HttpServer::run()`. It also possible to diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 1c8e4f053..ee3dae5d5 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,5 +1,11 @@ # Changes +# [Unreleased] + +### Fixed + +* Allow `SameSite=None` cookies to be sent in a response. + ## [1.0.1] - 2019-12-20 ### Fixed diff --git a/actix-http/src/cookie/draft.rs b/actix-http/src/cookie/draft.rs index a2b039121..1c7123422 100644 --- a/actix-http/src/cookie/draft.rs +++ b/actix-http/src/cookie/draft.rs @@ -10,18 +10,26 @@ use std::fmt; /// attribute is "Strict", then the cookie is never sent in cross-site requests. /// If the `SameSite` attribute is "Lax", the cookie is only sent in cross-site /// requests with "safe" HTTP methods, i.e, `GET`, `HEAD`, `OPTIONS`, `TRACE`. -/// If the `SameSite` attribute is not present (made explicit via the -/// `SameSite::None` variant), then the cookie will be sent as normal. +/// If the `SameSite` attribute is not present then the cookie will be sent as +/// normal. In some browsers, this will implicitly handle the cookie as if "Lax" +/// and in others, "None". It's best to explicitly set the `SameSite` attribute +/// to avoid inconsistent behavior. +/// +/// **Note:** Depending on browser, the `Secure` attribute may be required for +/// `SameSite` "None" cookies to be accepted. /// /// **Note:** This cookie attribute is an HTTP draft! Its meaning and definition /// are subject to change. +/// +/// More info about these draft changes can be found in the draft spec: +/// - https://tools.ietf.org/html/draft-west-cookie-incrementalism-00 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum SameSite { /// The "Strict" `SameSite` attribute. Strict, /// The "Lax" `SameSite` attribute. Lax, - /// No `SameSite` attribute. + /// The "None" `SameSite` attribute. None, } @@ -92,7 +100,7 @@ impl fmt::Display for SameSite { match *self { SameSite::Strict => write!(f, "Strict"), SameSite::Lax => write!(f, "Lax"), - SameSite::None => Ok(()), + SameSite::None => write!(f, "None"), } } } diff --git a/actix-http/src/cookie/mod.rs b/actix-http/src/cookie/mod.rs index 13fd5cf4e..d9db600ea 100644 --- a/actix-http/src/cookie/mod.rs +++ b/actix-http/src/cookie/mod.rs @@ -746,9 +746,7 @@ impl<'c> Cookie<'c> { } if let Some(same_site) = self.same_site() { - if !same_site.is_none() { - write!(f, "; SameSite={}", same_site)?; - } + write!(f, "; SameSite={}", same_site)?; } if let Some(path) = self.path() { @@ -1037,7 +1035,7 @@ mod tests { let cookie = Cookie::build("foo", "bar") .same_site(SameSite::None) .finish(); - assert_eq!(&cookie.to_string(), "foo=bar"); + assert_eq!(&cookie.to_string(), "foo=bar; SameSite=None"); } #[test] From c6fa007e726386544b0ebc8b7dd459d62bfd5e69 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Thu, 23 Jan 2020 11:27:34 +0900 Subject: [PATCH 3/9] Fix vcpkg cache (#1312) --- .github/workflows/windows.yml | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 52c82a886..06f5af824 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -3,7 +3,6 @@ name: CI (Windows) on: [push, pull_request] env: - VCPKG_HASH: 3f62e5d55d1a7d8905df35d5c441d6e9ad64ffdf VCPKGRS_DYNAMIC: 1 jobs: @@ -47,27 +46,15 @@ jobs: with: path: target key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-build-${{ hashFiles('**/Cargo.lock') }} - - name: Cache vcpkg package - uses: actions/cache@v1 - id: cache-vcpkg - with: - path: vcpkg - key: windows_x64-${{ env.VCPKG_HASH }}-vcpkg - name: Install OpenSSL - if: steps.cache-vcpkg.outputs.cache-hit != 'true' - shell: pwsh run: | - git clone https://github.com/Microsoft/vcpkg.git - cd vcpkg - git reset --hard $VCPKG_HASH - .\bootstrap-vcpkg.bat - .\vcpkg integrate install - .\vcpkg install openssl:x64-windows - Copy-Item .\installed\x64-windows\bin\libcrypto-1_1-x64.dll .\installed\x64-windows\bin\libcrypto.dll - Copy-Item .\installed\x64-windows\bin\libssl-1_1-x64.dll .\installed\x64-windows\bin\libssl.dll - Get-ChildItem .\installed\x64-windows\bin - Get-ChildItem .\installed\x64-windows\lib + vcpkg integrate install + vcpkg install openssl:x64-windows + Copy-Item C:\vcpkg\installed\x64-windows\bin\libcrypto-1_1-x64.dll C:\vcpkg\installed\x64-windows\bin\libcrypto.dll + Copy-Item C:\vcpkg\installed\x64-windows\bin\libssl-1_1-x64.dll C:\vcpkg\installed\x64-windows\bin\libssl.dll + Get-ChildItem C:\vcpkg\installed\x64-windows\bin + Get-ChildItem C:\vcpkg\installed\x64-windows\lib - name: check build uses: actions-rs/cargo@v1 From e17b3accb9327ab962af62599d37f78fd59e569b Mon Sep 17 00:00:00 2001 From: godofdream Date: Thu, 23 Jan 2020 21:10:02 +0100 Subject: [PATCH 4/9] Remove codecoverage for tests and examples (#1299) * Ignore Tests & Examples for CodeCoverage Ignore Tests & Examples for CodeCoverage --- codecov.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 codecov.yml diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 000000000..90cdfab47 --- /dev/null +++ b/codecov.yml @@ -0,0 +1,5 @@ +ignore: # ignore codecoverage on following paths + - "**/tests" + - "test-server" + - "**/benches" + - "**/examples" From 78f24dda037bf7f4350cfefca54a876a0e7ae162 Mon Sep 17 00:00:00 2001 From: cetra3 Date: Thu, 23 Jan 2020 22:32:34 +0000 Subject: [PATCH 5/9] Initial Issue template (#1311) * Initial Issue template * First round of changes for the bug report Co-authored-by: Yuki Okushi --- .github/ISSUE_TEMPLATE/bug_report.md | 32 ++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 000000000..0c4e6c1c6 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,32 @@ +Your issue may already be reported! +Please search on the [Actix Web issue tracker](https://github.com/actix/actix-web/issues) before creating one. + +## Expected Behavior + + + +## Current Behavior + + + +## Possible Solution + + + +## Steps to Reproduce (for bugs) + + +1. +2. +3. +4. + +## Context + + + +## Your Environment + + +* Rust Version (I.e, output of `rustc -V`): +* Actix Web Version: \ No newline at end of file From 58844874a0e56c51d964ab78f4d426da00541674 Mon Sep 17 00:00:00 2001 From: Maxim Vorobjov Date: Fri, 24 Jan 2020 07:51:38 +0200 Subject: [PATCH 6/9] Fixing #1295 convert UnsafeCell to RefCell in CloneableService (#1303) Co-authored-by: Yuki Okushi --- actix-http/src/cloneable.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/actix-http/src/cloneable.rs b/actix-http/src/cloneable.rs index 65c6bec21..c1dbfa430 100644 --- a/actix-http/src/cloneable.rs +++ b/actix-http/src/cloneable.rs @@ -1,4 +1,4 @@ -use std::cell::UnsafeCell; +use std::cell::RefCell; use std::rc::Rc; use std::task::{Context, Poll}; @@ -6,11 +6,15 @@ use actix_service::Service; #[doc(hidden)] /// Service that allows to turn non-clone service to a service with `Clone` impl -pub(crate) struct CloneableService(Rc>); +/// +/// # Panics +/// CloneableService might panic with some creative use of thread local storage. +/// See https://github.com/actix/actix-web/issues/1295 for example +pub(crate) struct CloneableService(Rc>); impl CloneableService { pub(crate) fn new(service: T) -> Self { - Self(Rc::new(UnsafeCell::new(service))) + Self(Rc::new(RefCell::new(service))) } } @@ -27,10 +31,10 @@ impl Service for CloneableService { type Future = T::Future; fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { - unsafe { &mut *self.0.as_ref().get() }.poll_ready(cx) + self.0.borrow_mut().poll_ready(cx) } fn call(&mut self, req: T::Request) -> Self::Future { - unsafe { &mut *self.0.as_ref().get() }.call(req) + self.0.borrow_mut().call(req) } } From cf3577550c04f6004f5fc33a57f7af814b0872cb Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Sat, 25 Jan 2020 02:27:13 +0900 Subject: [PATCH 7/9] Tweak caches (#1319) * Try to use `cargo-cache` * Tweak issue template --- .github/ISSUE_TEMPLATE/bug_report.md | 7 ++++++- .github/workflows/bench.yml | 11 ++++++++--- .github/workflows/macos.yml | 11 ++++++++--- .github/workflows/windows.yml | 11 ++++++++--- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 0c4e6c1c6..128f51ffd 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -1,3 +1,8 @@ +--- +name: bug report +about: create a bug report +--- + Your issue may already be reported! Please search on the [Actix Web issue tracker](https://github.com/actix/actix-web/issues) before creating one. @@ -29,4 +34,4 @@ Please search on the [Actix Web issue tracker](https://github.com/actix/actix-we * Rust Version (I.e, output of `rustc -V`): -* Actix Web Version: \ No newline at end of file +* Actix Web Version: diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 3c9deb49c..08bb81d48 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -24,19 +24,24 @@ jobs: uses: actions/cache@v1 with: path: ~/.cargo/registry - key: ${{ matrix.version }}-x86_64-unknown-linux-gnu-registry-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-unknown-linux-gnu-registry-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Cache cargo index uses: actions/cache@v1 with: path: ~/.cargo/git - key: ${{ matrix.version }}-x86_64-unknown-linux-gnu-cargo-index-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-unknown-linux-gnu-cargo-index-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Cache cargo build uses: actions/cache@v1 with: path: target - key: ${{ matrix.version }}-x86_64-unknown-linux-gnu-cargo-build-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-unknown-linux-gnu-cargo-build-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Check benchmark uses: actions-rs/cargo@v1 with: command: bench + + - name: Clear the cargo caches + run: | + cargo install cargo-cache --no-default-features --features ci-autoclean + cargo-cache diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index d30d6b3e9..397236a29 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -32,17 +32,17 @@ jobs: uses: actions/cache@v1 with: path: ~/.cargo/registry - key: ${{ matrix.version }}-x86_64-apple-darwin-cargo-registry-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-apple-darwin-cargo-registry-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Cache cargo index uses: actions/cache@v1 with: path: ~/.cargo/git - key: ${{ matrix.version }}-x86_64-apple-darwin-cargo-index-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-apple-darwin-cargo-index-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Cache cargo build uses: actions/cache@v1 with: path: target - key: ${{ matrix.version }}-x86_64-apple-darwin-cargo-build-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-apple-darwin-cargo-build-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: check build uses: actions-rs/cargo@v1 @@ -57,3 +57,8 @@ jobs: args: --all --all-features --no-fail-fast -- --nocapture --skip=test_h2_content_length --skip=test_reading_deflate_encoding_large_random_rustls + + - name: Clear the cargo caches + run: | + cargo install cargo-cache --no-default-features --features ci-autoclean + cargo-cache diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 06f5af824..fed4ce031 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -35,17 +35,17 @@ jobs: uses: actions/cache@v1 with: path: ~/.cargo/registry - key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-registry-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-registry-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Cache cargo index uses: actions/cache@v1 with: path: ~/.cargo/git - key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-index-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-index-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Cache cargo build uses: actions/cache@v1 with: path: target - key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-build-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-build-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Install OpenSSL run: | @@ -74,3 +74,8 @@ jobs: --skip=test_expect_continue --skip=test_http10_keepalive --skip=test_slow_request + + - name: Clear the cargo caches + run: | + cargo install cargo-cache --no-default-features --features ci-autoclean + cargo-cache From 8888520d83b43ccd217f1cfb9ba764a39cb72668 Mon Sep 17 00:00:00 2001 From: Maxim Vorobjov Date: Sat, 25 Jan 2020 01:05:25 +0200 Subject: [PATCH 8/9] Add benchmark for full stack request lifecycle (#1298) * add benchmark for full stack request lifecycle * add direct service benchmarks * fix newline * add cloneable service benchmarks * remove cloneable bench experiments + cargo fmt Co-authored-by: Yuki Okushi --- Cargo.toml | 9 +++ actix-http/src/cloneable.rs | 2 +- actix-http/src/cookie/draft.rs | 4 +- benches/server.rs | 64 +++++++++++++++++++ benches/service.rs | 108 +++++++++++++++++++++++++++++++++ 5 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 benches/server.rs create mode 100644 benches/service.rs diff --git a/Cargo.toml b/Cargo.toml index 9e1b559eb..9f0748e0c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -99,6 +99,7 @@ env_logger = "0.6" serde_derive = "1.0" brotli2 = "0.3.2" flate2 = "1.0.13" +criterion = "0.3" [profile.release] lto = true @@ -116,3 +117,11 @@ actix-session = { path = "actix-session" } actix-files = { path = "actix-files" } actix-multipart = { path = "actix-multipart" } awc = { path = "awc" } + +[[bench]] +name = "server" +harness = false + +[[bench]] +name = "service" +harness = false diff --git a/actix-http/src/cloneable.rs b/actix-http/src/cloneable.rs index c1dbfa430..b64c299fc 100644 --- a/actix-http/src/cloneable.rs +++ b/actix-http/src/cloneable.rs @@ -6,7 +6,7 @@ use actix_service::Service; #[doc(hidden)] /// Service that allows to turn non-clone service to a service with `Clone` impl -/// +/// /// # Panics /// CloneableService might panic with some creative use of thread local storage. /// See https://github.com/actix/actix-web/issues/1295 for example diff --git a/actix-http/src/cookie/draft.rs b/actix-http/src/cookie/draft.rs index 1c7123422..a6525a605 100644 --- a/actix-http/src/cookie/draft.rs +++ b/actix-http/src/cookie/draft.rs @@ -14,13 +14,13 @@ use std::fmt; /// normal. In some browsers, this will implicitly handle the cookie as if "Lax" /// and in others, "None". It's best to explicitly set the `SameSite` attribute /// to avoid inconsistent behavior. -/// +/// /// **Note:** Depending on browser, the `Secure` attribute may be required for /// `SameSite` "None" cookies to be accepted. /// /// **Note:** This cookie attribute is an HTTP draft! Its meaning and definition /// are subject to change. -/// +/// /// More info about these draft changes can be found in the draft spec: /// - https://tools.ietf.org/html/draft-west-cookie-incrementalism-00 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] diff --git a/benches/server.rs b/benches/server.rs new file mode 100644 index 000000000..93079a223 --- /dev/null +++ b/benches/server.rs @@ -0,0 +1,64 @@ +use actix_web::{test, web, App, HttpResponse}; +use awc::Client; +use criterion::{criterion_group, criterion_main, Criterion}; +use futures::future::join_all; + +const STR: &str = "Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World"; + +// benchmark sending all requests at the same time +fn bench_async_burst(c: &mut Criterion) { + let srv = test::start(|| { + App::new() + .service(web::resource("/").route(web::to(|| HttpResponse::Ok().body(STR)))) + }); + + // We are using System here, since Runtime requires preinitialized tokio + // Maybe add to actix_rt docs + let url = srv.url("/"); + let mut rt = actix_rt::System::new("test"); + + c.bench_function("get_body_async_burst", move |b| { + b.iter_custom(|iters| { + let client = Client::new().get(url.clone()).freeze().unwrap(); + + let start = std::time::Instant::now(); + // benchmark body + let resps = rt.block_on(async move { + let burst = (0..iters).map(|_| client.send()); + join_all(burst).await + }); + let elapsed = start.elapsed(); + + // if there are failed requests that might be an issue + let failed = resps.iter().filter(|r| r.is_err()).count(); + if failed > 0 { + eprintln!("failed {} requests (might be bench timeout)", failed); + }; + + elapsed + }) + }); +} + +criterion_group!(server_benches, bench_async_burst); +criterion_main!(server_benches); diff --git a/benches/service.rs b/benches/service.rs new file mode 100644 index 000000000..8adbc8a0c --- /dev/null +++ b/benches/service.rs @@ -0,0 +1,108 @@ +use actix_service::Service; +use actix_web::dev::{ServiceRequest, ServiceResponse}; +use actix_web::{web, App, Error, HttpResponse}; +use criterion::{criterion_main, Criterion}; +use std::cell::RefCell; +use std::rc::Rc; + +use actix_web::test::{init_service, ok_service, TestRequest}; + +/// Criterion Benchmark for async Service +/// Should be used from within criterion group: +/// ```rust,ignore +/// let mut criterion: ::criterion::Criterion<_> = +/// ::criterion::Criterion::default().configure_from_args(); +/// bench_async_service(&mut criterion, ok_service(), "async_service_direct"); +/// ``` +/// +/// Usable for benching Service wrappers: +/// Using minimum service code implementation we first measure +/// time to run minimum service, then measure time with wrapper. +/// +/// Sample output +/// async_service_direct time: [1.0908 us 1.1656 us 1.2613 us] +pub fn bench_async_service(c: &mut Criterion, srv: S, name: &str) +where + S: Service + + 'static, +{ + let mut rt = actix_rt::System::new("test"); + let srv = Rc::new(RefCell::new(srv)); + + let req = TestRequest::default().to_srv_request(); + assert!(rt + .block_on(srv.borrow_mut().call(req)) + .unwrap() + .status() + .is_success()); + + // start benchmark loops + c.bench_function(name, move |b| { + b.iter_custom(|iters| { + let srv = srv.clone(); + // exclude request generation, it appears it takes significant time vs call (3us vs 1us) + let reqs: Vec<_> = (0..iters) + .map(|_| TestRequest::default().to_srv_request()) + .collect(); + let start = std::time::Instant::now(); + // benchmark body + rt.block_on(async move { + for req in reqs { + srv.borrow_mut().call(req).await.unwrap(); + } + }); + let elapsed = start.elapsed(); + // check that at least first request succeeded + elapsed + }) + }); +} + +async fn index(req: ServiceRequest) -> Result { + Ok(req.into_response(HttpResponse::Ok().finish())) +} + +// Benchmark basic WebService directly +// this approach is usable for benching WebService, though it adds some time to direct service call: +// Sample results on MacBook Pro '14 +// time: [2.0724 us 2.1345 us 2.2074 us] +fn async_web_service(c: &mut Criterion) { + let mut rt = actix_rt::System::new("test"); + let srv = Rc::new(RefCell::new(rt.block_on(init_service( + App::new().service(web::service("/").finish(index)), + )))); + + let req = TestRequest::get().uri("/").to_request(); + assert!(rt + .block_on(srv.borrow_mut().call(req)) + .unwrap() + .status() + .is_success()); + + // start benchmark loops + c.bench_function("async_web_service_direct", move |b| { + b.iter_custom(|iters| { + let srv = srv.clone(); + let reqs = (0..iters).map(|_| TestRequest::get().uri("/").to_request()); + + let start = std::time::Instant::now(); + // benchmark body + rt.block_on(async move { + for req in reqs { + srv.borrow_mut().call(req).await.unwrap(); + } + }); + let elapsed = start.elapsed(); + // check that at least first request succeeded + elapsed + }) + }); +} + +pub fn service_benches() { + let mut criterion: ::criterion::Criterion<_> = + ::criterion::Criterion::default().configure_from_args(); + bench_async_service(&mut criterion, ok_service(), "async_service_direct"); + async_web_service(&mut criterion); +} +criterion_main!(service_benches); From 71d11644a71b243f8f4506b715db4f7c5dddbae1 Mon Sep 17 00:00:00 2001 From: Andrey Torsunov Date: Sun, 26 Jan 2020 01:22:40 +0300 Subject: [PATCH 9/9] Add ability to name a handler function as 'config' (#1290) * eliminate handler naming restrictions #1277 * Update actix-web-codegen/CHANGES.md Co-authored-by: Yuki Okushi --- actix-web-codegen/CHANGES.md | 4 ++++ actix-web-codegen/src/route.rs | 6 +++--- actix-web-codegen/tests/test_macro.rs | 6 ++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/actix-web-codegen/CHANGES.md b/actix-web-codegen/CHANGES.md index de676f688..95696abd3 100644 --- a/actix-web-codegen/CHANGES.md +++ b/actix-web-codegen/CHANGES.md @@ -1,5 +1,9 @@ # Changes +## [0.2.NEXT] - 2020-xx-xx + +* Allow the handler function to be named as `config` #1290 + ## [0.2.0] - 2019-12-13 * Generate code for actix-web 2.0 diff --git a/actix-web-codegen/src/route.rs b/actix-web-codegen/src/route.rs index 16d3e8157..d48198484 100644 --- a/actix-web-codegen/src/route.rs +++ b/actix-web-codegen/src/route.rs @@ -195,15 +195,15 @@ impl Route { pub struct #name; impl actix_web::dev::HttpServiceFactory for #name { - fn register(self, config: &mut actix_web::dev::AppService) { + fn register(self, __config: &mut actix_web::dev::AppService) { #ast - let resource = actix_web::Resource::new(#path) + let __resource = actix_web::Resource::new(#path) .name(#resource_name) .guard(actix_web::guard::#guard()) #(.guard(actix_web::guard::fn_guard(#extra_guards)))* .#resource_type(#name); - actix_web::dev::HttpServiceFactory::register(resource, config) + actix_web::dev::HttpServiceFactory::register(__resource, __config) } } }; diff --git a/actix-web-codegen/tests/test_macro.rs b/actix-web-codegen/tests/test_macro.rs index 4ac1a8023..ffb50c11e 100644 --- a/actix-web-codegen/tests/test_macro.rs +++ b/actix-web-codegen/tests/test_macro.rs @@ -2,6 +2,12 @@ use actix_web::{http, test, web::Path, App, HttpResponse, Responder}; use actix_web_codegen::{connect, delete, get, head, options, patch, post, put, trace}; use futures::{future, Future}; +// Make sure that we can name function as 'config' +#[get("/config")] +async fn config() -> impl Responder { + HttpResponse::Ok() +} + #[get("/test")] async fn test_handler() -> impl Responder { HttpResponse::Ok()