From d8c059871721e8b61f19fa441c3951517b9f2ede Mon Sep 17 00:00:00 2001 From: Richa Jain Date: Wed, 25 Oct 2023 23:45:20 +0800 Subject: [PATCH] Some code refactor based on Taiki's feedback --- 0001-CI-split-extra-step-into-two.patch | 88 ------------------------- pre-commit | 2 +- scripts/collect_steps.py | 58 ++++++++-------- src/protocol/step/steps.txt | 4 ++ steps1.txt | 1 - 5 files changed, 35 insertions(+), 118 deletions(-) delete mode 100644 0001-CI-split-extra-step-into-two.patch delete mode 100644 steps1.txt diff --git a/0001-CI-split-extra-step-into-two.patch b/0001-CI-split-extra-step-into-two.patch deleted file mode 100644 index 85d7146d2..000000000 --- a/0001-CI-split-extra-step-into-two.patch +++ /dev/null @@ -1,88 +0,0 @@ -From ba6c503fbe930a9298d83c04bab15f329faf13ae Mon Sep 17 00:00:00 2001 -From: Alex Koshelev -Date: Fri, 11 Aug 2023 14:25:03 -0700 -Subject: [PATCH] CI: split extra step into two - -This is an attempt to increase parallelism on CI. Those builds take ~4 mins and running tests take another 5-6 mins. Splitting this step should make CI run faster. ---- - .github/workflows/check.yml | 42 ++++++++++++++++++++++++++++--------- - 1 file changed, 32 insertions(+), 10 deletions(-) - -diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml -index 54f869ac..9a8e1886 100644 ---- a/.github/workflows/check.yml -+++ b/.github/workflows/check.yml -@@ -15,13 +15,15 @@ on: - - "benches/**/*" - - "tests/**/*" - -+env: -+ CARGO_TERM_COLOR: always -+ RUSTFLAGS: -D warnings -+ RUSTDOCFLAGS: -D warnings -+ - jobs: - basic: - name: Basic Checks - env: -- CARGO_TERM_COLOR: always -- RUSTFLAGS: -D warnings -- RUSTDOCFLAGS: -D warnings - CARGO_INCREMENTAL: 0 - - runs-on: ubuntu-latest -@@ -64,12 +66,13 @@ jobs: - - name: Run Web Tests - run: cargo test --no-default-features --features "cli web-app real-world-infra test-fixture descriptive-gate" - -- extra: -- name: Additional Builds and Concurrency Tests -+ - name: Run compact gate tests -+ run: cargo test --no-default-features --features "cli web-app real-world-infra test-fixture compact-gate" -+ -+ extra-builds: -+ name: Additional Builds - env: -- CARGO_TERM_COLOR: always - RUSTFLAGS: -D warnings -C target-cpu=native -- RUSTDOCFLAGS: -D warnings - - runs-on: ubuntu-latest - -@@ -102,11 +105,30 @@ jobs: - - name: Build concurrency tests (debug mode) - run: cargo build --features shuttle - -+ benches-and-fuzzy: -+ name: Run benchmarks and concurrency tests -+ -+ runs-on: ubuntu-latest -+ -+ steps: -+ - uses: actions/checkout@v3 -+ -+ - uses: dtolnay/rust-toolchain@stable -+ with: -+ components: clippy,rustfmt -+ -+ - uses: actions/cache@v3 -+ with: -+ path: | -+ ~/.cargo/bin/ -+ ~/.cargo/registry/index/ -+ ~/.cargo/registry/cache/ -+ ~/.cargo/git/db/ -+ target/ -+ key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }} -+ - - name: Run concurrency tests - run: cargo test --release --features shuttle - - - name: Run IPA bench - run: cargo bench --bench oneshot_ipa --no-default-features --features "enable-benches descriptive-gate" -- -- - name: Run compact gate tests -- run: cargo test --no-default-features --features "cli web-app real-world-infra test-fixture compact-gate" --- -2.31.1 - diff --git a/pre-commit b/pre-commit index 40dcbf745..d60bece9e 100755 --- a/pre-commit +++ b/pre-commit @@ -100,7 +100,7 @@ check "IPA benchmark" \ cargo bench --bench oneshot_ipa --no-default-features --features="enable-benches descriptive-gate" -- -n 62 check "IPA OPRF benchmark" \ - cargo bench --bench oneshot_ipa --no-default-features --features="enable-benches descriptive-gate" -- -n 62 --oprf + cargo bench --bench oneshot_ipa --no-default-features --features="enable-benches descriptive-gate" -- -n 62 --oprf -c 16 check "Arithmetic circuit benchmark" \ cargo bench --bench oneshot_arithmetic --no-default-features --features "enable-benches descriptive-gate" diff --git a/scripts/collect_steps.py b/scripts/collect_steps.py index 3e1371ef2..23c8ffc5e 100755 --- a/scripts/collect_steps.py +++ b/scripts/collect_steps.py @@ -149,7 +149,8 @@ def extract_intermediate_steps(steps): return steps -def ipa_steps(steps): +def ipa_steps(): + output = set() for c in PER_USER_CAP: for w in ATTRIBUTION_WINDOW: for b in BREAKDOWN_KEYS: @@ -167,41 +168,42 @@ def ipa_steps(steps): m, ] print(" ".join(args), file=sys.stderr) - steps.update(collect_steps(args)) + output.update(collect_steps(args)) + return output -OPRF_BREAKDOWN_KEYS = [256] +OPRF_BREAKDOWN_KEY = 256 OPRF_USER_CAP = [16, 64, 128] -OPRF_SECURITY_MODEL = ["semi-honest"] -OPRF_TRIGGER_VALUE = [7] +OPRF_SECURITY_MODEL = "semi-honest" +OPRF_TRIGGER_VALUE = [6, 7] -def oprf_steps(steps): +def oprf_steps(): + output = set() for c in OPRF_USER_CAP: for w in ATTRIBUTION_WINDOW: - for b in OPRF_BREAKDOWN_KEYS: - for m in OPRF_SECURITY_MODEL: - for tv in OPRF_TRIGGER_VALUE: - args = ARGS + [ - "-n", - str(QUERY_SIZE), - "-c", - str(c), - "-w", - str(w), - "-b", - str(b), - "-m", - m, - "-t", - str(tv), - "-o" - ] - print(" ".join(args), file=sys.stderr) - steps.update(collect_steps(args)) + for tv in OPRF_TRIGGER_VALUE: + args = ARGS + [ + "-n", + str(QUERY_SIZE), + "-c", + str(c), + "-w", + str(w), + "-b", + str(OPRF_BREAKDOWN_KEY), + "-m", + OPRF_SECURITY_MODEL, + "-t", + str(tv), + "-o" + ] + print(" ".join(args), file=sys.stderr) + output.update(collect_steps(args)) + return output if __name__ == "__main__": steps = set() - ipa_steps(steps) - oprf_steps(steps) + steps.update(ipa_steps()) + steps.update(oprf_steps()) full_steps = extract_intermediate_steps(steps) sorted_steps = sorted(full_steps) diff --git a/src/protocol/step/steps.txt b/src/protocol/step/steps.txt index 2079b4ee3..c5bafcded 100644 --- a/src/protocol/step/steps.txt +++ b/src/protocol/step/steps.txt @@ -9345,6 +9345,7 @@ ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding: ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row2/ipa::protocol::prf_sharding::Step::compute_saturating_sum/ipa::protocol::step::BitOpStep::bit4 ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row2/ipa::protocol::prf_sharding::Step::compute_saturating_sum/ipa::protocol::step::BitOpStep::bit5 ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row2/ipa::protocol::prf_sharding::Step::compute_saturating_sum/ipa::protocol::step::BitOpStep::bit6 +ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row2/ipa::protocol::prf_sharding::Step::compute_saturating_sum/ipa::protocol::step::BitOpStep::bit7 ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row2/ipa::protocol::prf_sharding::Step::computed_capped_attributed_trigger_value_just_saturated_case ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row2/ipa::protocol::prf_sharding::Step::computed_capped_attributed_trigger_value_just_saturated_case/ipa::protocol::step::BitOpStep::bit0 ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row2/ipa::protocol::prf_sharding::Step::computed_capped_attributed_trigger_value_just_saturated_case/ipa::protocol::step::BitOpStep::bit1 @@ -9446,6 +9447,9 @@ ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding: ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row3/ipa::protocol::prf_sharding::Step::compute_saturating_sum/ipa::protocol::step::BitOpStep::bit2 ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row3/ipa::protocol::prf_sharding::Step::compute_saturating_sum/ipa::protocol::step::BitOpStep::bit3 ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row3/ipa::protocol::prf_sharding::Step::compute_saturating_sum/ipa::protocol::step::BitOpStep::bit4 +ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row3/ipa::protocol::prf_sharding::Step::compute_saturating_sum/ipa::protocol::step::BitOpStep::bit5 +ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row3/ipa::protocol::prf_sharding::Step::compute_saturating_sum/ipa::protocol::step::BitOpStep::bit6 +ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row3/ipa::protocol::prf_sharding::Step::compute_saturating_sum/ipa::protocol::step::BitOpStep::bit7 ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row3/ipa::protocol::prf_sharding::Step::computed_capped_attributed_trigger_value_just_saturated_case ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row3/ipa::protocol::prf_sharding::Step::computed_capped_attributed_trigger_value_just_saturated_case/ipa::protocol::step::BitOpStep::bit0 ipa::protocol::prf_sharding::Step::binary_validator/ipa::protocol::prf_sharding::UserNthRowStep::row3/ipa::protocol::prf_sharding::Step::computed_capped_attributed_trigger_value_just_saturated_case/ipa::protocol::step::BitOpStep::bit1 diff --git a/steps1.txt b/steps1.txt deleted file mode 100644 index ff3d9f397..000000000 --- a/steps1.txt +++ /dev/null @@ -1 +0,0 @@ -No steps in the output