From 3e7ba58439a434be5e6894993370a8cef403303e Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 6 Feb 2024 17:59:37 +0100 Subject: [PATCH] Delete `EncryptionPerformanceTest` On my machine, running the whole Active Record test suite takes `88` seconds, and `40` of these are spent in encryption tests. Some of them also happen to flake because of random blips. I appreciate the care that has been put into ensuring the overhead of encrption was reasonable, but I don't think these tests justify their cost. --- Gemfile | 2 - Gemfile.lock | 2 - activerecord/test/cases/encryption/helper.rb | 42 +---------- .../encryption_performance_test.rb | 42 ----------- .../envelope_encryption_performance_test.rb | 53 -------------- ..._deterministic_queries_performance_test.rb | 23 ------ .../performance/storage_performance_test.rb | 70 ------------------- 7 files changed, 1 insertion(+), 233 deletions(-) delete mode 100644 activerecord/test/cases/encryption/performance/encryption_performance_test.rb delete mode 100644 activerecord/test/cases/encryption/performance/envelope_encryption_performance_test.rb delete mode 100644 activerecord/test/cases/encryption/performance/extended_deterministic_queries_performance_test.rb delete mode 100644 activerecord/test/cases/encryption/performance/storage_performance_test.rb diff --git a/Gemfile b/Gemfile index cf2f1b9f4b..f6fc8c1bc9 100644 --- a/Gemfile +++ b/Gemfile @@ -143,8 +143,6 @@ group :test do gem "debug", ">= 1.1.0", require: false end - gem "benchmark-ips" - # Needed for Railties tests because it is included in generated apps. gem "brakeman" end diff --git a/Gemfile.lock b/Gemfile.lock index 7ca2f07a10..f0cc4b9d35 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -159,7 +159,6 @@ GEM base64 (0.2.0) bcrypt (3.1.20) beaneater (1.1.3) - benchmark-ips (2.13.0) bigdecimal (3.1.5) bindex (0.8.1) bootsnap (1.17.0) @@ -596,7 +595,6 @@ DEPENDENCIES azure-storage-blob (~> 2.0) backburner bcrypt (~> 3.1.11) - benchmark-ips bootsnap (>= 1.4.4) brakeman capybara (>= 3.39) diff --git a/activerecord/test/cases/encryption/helper.rb b/activerecord/test/cases/encryption/helper.rb index 1913a124d6..1839bff03e 100644 --- a/activerecord/test/cases/encryption/helper.rb +++ b/activerecord/test/cases/encryption/helper.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "cases/helper" -require "benchmark/ips" class ActiveRecord::Fixture prepend ActiveRecord::Encryption::EncryptedFixtures @@ -111,45 +110,6 @@ def assert_invalid_key_cant_read_attribute_with_custom_key_provider(model, attri attribute_type.key_provider.keys = original_keys end end - - module PerformanceHelpers - BENCHMARK_DURATION = 1 - BENCHMARK_WARMUP = 1 - BASELINE_LABEL = "Baseline" - CODE_TO_TEST_LABEL = "Code" - - # Usage: - # - # baseline = -> { } - # - # assert_slower_by_at_most 2, baseline: baseline do - # - # end - def assert_slower_by_at_most(threshold_factor, baseline:, baseline_label: BASELINE_LABEL, code_to_test_label: CODE_TO_TEST_LABEL, duration: BENCHMARK_DURATION, quiet: true, &block_to_test) - GC.start - - result = nil - output, error = capture_io do - result = Benchmark.ips do |x| - x.config(time: duration, warmup: BENCHMARK_WARMUP) - x.report(code_to_test_label, &block_to_test) - x.report(baseline_label, &baseline) - x.compare! - end - end - - baseline_result = result.entries.find { |entry| entry.label == baseline_label } - code_to_test_result = result.entries.find { |entry| entry.label == code_to_test_label } - - times_slower = baseline_result.ips / code_to_test_result.ips - - if !quiet || times_slower >= threshold_factor - puts "#{output}#{error}" - end - - assert times_slower < threshold_factor, "Expecting #{threshold_factor} times slower at most, but got #{times_slower} times slower" - end - end end # We eager load encrypted attribute types as they are declared, so that they pick up the @@ -163,7 +123,7 @@ def assert_slower_by_at_most(threshold_factor, baseline:, baseline_label: BASELI end class ActiveRecord::EncryptionTestCase < ActiveRecord::TestCase - include ActiveRecord::Encryption::EncryptionHelpers, ActiveRecord::Encryption::PerformanceHelpers + include ActiveRecord::Encryption::EncryptionHelpers ENCRYPTION_PROPERTIES_TO_RESET = { config: %i[ primary_key deterministic_key key_derivation_salt store_key_references hash_digest_class diff --git a/activerecord/test/cases/encryption/performance/encryption_performance_test.rb b/activerecord/test/cases/encryption/performance/encryption_performance_test.rb deleted file mode 100644 index 54ac5fc8df..0000000000 --- a/activerecord/test/cases/encryption/performance/encryption_performance_test.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true - -require "cases/encryption/helper" -require "models/book_encrypted" -require "models/post_encrypted" - -class ActiveRecord::Encryption::EncryptionPerformanceTest < ActiveRecord::EncryptionTestCase - fixtures :encrypted_books, :posts - - setup do - ActiveRecord::Encryption.config.support_unencrypted_data = true - end - - test "performance when saving records" do - baseline = -> { create_post_without_encryption } - - assert_slower_by_at_most 1.4, baseline: baseline do - create_post_with_encryption - end - end - - test "reading an encrypted attribute multiple times is as fast as reading a regular attribute" do - unencrypted_post = create_post_without_encryption - baseline = -> { unencrypted_post.reload.title } - - encrypted_post = create_post_with_encryption - assert_slower_by_at_most 1.2, baseline: baseline, duration: 3 do - encrypted_post.reload.title - end - end - - private - def create_post_without_encryption - ActiveRecord::Encryption.without_encryption { create_post_with_encryption } - end - - def create_post_with_encryption - EncryptedPost.create!\ - title: "the Starfleet is here!", - body: "

the Starfleet is here, we are safe now!

" - end -end diff --git a/activerecord/test/cases/encryption/performance/envelope_encryption_performance_test.rb b/activerecord/test/cases/encryption/performance/envelope_encryption_performance_test.rb deleted file mode 100644 index 85b7f0abf7..0000000000 --- a/activerecord/test/cases/encryption/performance/envelope_encryption_performance_test.rb +++ /dev/null @@ -1,53 +0,0 @@ -# frozen_string_literal: true - -require "cases/encryption/helper" -require "models/book_encrypted" - -class ActiveRecord::Encryption::EnvelopeEncryptionPerformanceTest < ActiveRecord::EncryptionTestCase - fixtures :encrypted_books - - setup do - ActiveRecord::Encryption.config.support_unencrypted_data = true - @envelope_encryption_key_provider = ActiveRecord::Encryption::EnvelopeEncryptionKeyProvider.new - end - - test "performance when saving records" do - baseline = -> { create_book_without_encryption } - - assert_slower_by_at_most 1.4, baseline: baseline do - with_envelope_encryption do - create_book - end - end - end - - test "reading an encrypted attribute multiple times is as fast as reading a regular attribute" do - with_envelope_encryption do - baseline = -> { encrypted_books(:awdr).created_at } - book = create_book - assert_slower_by_at_most 1.05, baseline: baseline, duration: 3 do - book.name - end - end - end - - private - def create_book_without_encryption - ActiveRecord::Encryption.without_encryption { create_book } - end - - def create_book - EncryptedBook.create! name: "Dune" - end - - def encrypt_unencrypted_book - book = create_book_without_encryption - with_envelope_encryption do - book.encrypt - end - end - - def with_envelope_encryption(&block) - with_key_provider @envelope_encryption_key_provider, &block - end -end diff --git a/activerecord/test/cases/encryption/performance/extended_deterministic_queries_performance_test.rb b/activerecord/test/cases/encryption/performance/extended_deterministic_queries_performance_test.rb deleted file mode 100644 index 2f853c6565..0000000000 --- a/activerecord/test/cases/encryption/performance/extended_deterministic_queries_performance_test.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -require "cases/encryption/helper" -require "models/book_encrypted" - -class ActiveRecord::Encryption::ExtendedDeterministicQueriesPerformanceTest < ActiveRecord::EncryptionTestCase - test "finding with prepared statement caching by deterministically encrypted columns" do - baseline = -> { EncryptedBook.find_by(format: "paperback") } # not encrypted - - assert_slower_by_at_most 1.7, baseline: baseline, duration: 2 do - EncryptedBook.find_by(name: "Agile Web Development with Rails") # encrypted, deterministic - end - end - - test "finding without prepared statement caching by encrypted columns (deterministic)" do - baseline = -> { EncryptedBook.where("id > 0").find_by(format: "paperback") } # not encrypted - - # Overhead is 1.1 with SQL - assert_slower_by_at_most 1.3, baseline: baseline, duration: 2 do - EncryptedBook.where("id > 0").find_by(name: "Agile Web Development with Rails") # encrypted, deterministic - end - end -end diff --git a/activerecord/test/cases/encryption/performance/storage_performance_test.rb b/activerecord/test/cases/encryption/performance/storage_performance_test.rb deleted file mode 100644 index d777ec9f15..0000000000 --- a/activerecord/test/cases/encryption/performance/storage_performance_test.rb +++ /dev/null @@ -1,70 +0,0 @@ -# frozen_string_literal: true - -require "cases/encryption/helper" - -class ActiveRecord::Encryption::StoragePerformanceTest < ActiveRecord::EncryptionTestCase - test "storage overload without storing keys is acceptable" do - assert_storage_performance size: 2, overload_less_than: 47 - assert_storage_performance size: 50, overload_less_than: 4 - assert_storage_performance size: 255, overload_less_than: 1.6 - assert_storage_performance size: 1.kilobyte, overload_less_than: 1.15 - - [500.kilobytes, 1.megabyte, 10.megabyte].each do |size| - assert_storage_performance size: size, overload_less_than: 1.03 - end - end - - test "storage overload storing keys is acceptable for DerivedSecretKeyProvider" do - ActiveRecord::Encryption.config.store_key_references = true - - ActiveRecord::Encryption.with_encryption_context key_provider: ActiveRecord::Encryption::DerivedSecretKeyProvider.new("some secret") do - assert_storage_performance size: 2, overload_less_than: 54 - assert_storage_performance size: 50, overload_less_than: 3.5 - assert_storage_performance size: 255, overload_less_than: 1.64 - assert_storage_performance size: 1.kilobyte, overload_less_than: 1.18 - - [500.kilobytes, 1.megabyte, 10.megabyte].each do |size| - assert_storage_performance size: size, overload_less_than: 1.03 - end - end - end - - test "storage overload storing keys is acceptable for EnvelopeEncryptionKeyProvider" do - ActiveRecord::Encryption.config.store_key_references = true - - with_envelope_encryption do - assert_storage_performance size: 2, overload_less_than: 126 - assert_storage_performance size: 50, overload_less_than: 6.28 - assert_storage_performance size: 255, overload_less_than: 2.3 - assert_storage_performance size: 1.kilobyte, overload_less_than: 1.3 - - [500.kilobytes, 1.megabyte, 10.megabyte].each do |size| - assert_storage_performance size: size, overload_less_than: 1.015 - end - end - end - - private - def assert_storage_performance(size:, overload_less_than:, quiet: true) - clear_content = SecureRandom.urlsafe_base64(size).first(size) # .alphanumeric is very slow for large sizes - encrypted_content = encryptor.encrypt(clear_content) - - overload_factor = encrypted_content.bytesize.to_f / clear_content.bytesize - - if !quiet || overload_factor > overload_less_than - puts "#{clear_content.bytesize}; #{encrypted_content.bytesize}; #{(encrypted_content.bytesize / clear_content.bytesize.to_f)}" - end - - assert\ - overload_factor <= overload_less_than, - "Expecting a storage overload of #{overload_less_than} at most for #{size} bytes, but got #{overload_factor} instead" - end - - def encryptor - @encryptor ||= ActiveRecord::Encryption::Encryptor.new - end - - def cipher - @cipher ||= ActiveRecord::Encryption::Cipher.new - end -end