services: cache zxcvbn dictionaries per-thread

Before, every time a password was tested, the dictionaries were parsed
again by zxcvbn.

Parsing dictionaries is slow: it may take up to ~1s. This doesn't matter
that much in production, but it makes tests very slow (because we tend
to create a lot of User records).

With this changes, the initializer tester is shared between calls, class
instances and threads. It is lazily loaded on first use, in order not to
slow down the application boot sequence.

This uses ~20 Mo of memory (only once for all threads), but makes tests
more that twice faster.

For instance, model tests go from **8m 21s** to **3m 26s**.

NB:
An additionnal optimization could be to preload the tester on
boot, before workers are forked, to take advantage of Puma copy-on-write
mechanism. In this way all forked workers would use the same cached
instance.

But:

- We're not actually sure this would work properly. What if Ruby updates
  an interval ivar on the class, and this forces the OS to copy the
  whole data structure in each fork?
- Puma phased restarts are not compatible with copy-on-write anyway.

So we're avoiding this optimisation for now, and take the extra 20 Mo
per worker.
This commit is contained in:
Pierre de La Morinerie 2021-10-25 09:57:01 +00:00
parent 5818841daa
commit d0e87a08cf
3 changed files with 76 additions and 2 deletions

View file

@ -1,4 +1,32 @@
class ZxcvbnService class ZxcvbnService
@tester_mutex = Mutex.new
class << self
# Returns an Zxcvbn instance cached between classes instances and between threads.
#
# The tester weights ~20 Mo, and we'd like to save some memory so rather
# that storing it in a per-thread accessor, we prefer to use a mutex
# to cache it between threads.
def tester
@tester_mutex.synchronize do
@tester ||= build_tester
end
end
private
# Returns a fully initializer tester from the on-disk dictionary.
#
# This is slow: loading and parsing the dictionary may take around 1s.
def build_tester
dictionaries = YAML.safe_load(File.read(Rails.root.join("config", "initializers", "zxcvbn_dictionnaries.yaml")))
tester = Zxcvbn::Tester.new
tester.add_word_lists(dictionaries)
tester
end
end
def initialize(password) def initialize(password)
@password = password @password = password
end end
@ -18,6 +46,6 @@ class ZxcvbnService
private private
def compute_zxcvbn def compute_zxcvbn
Zxcvbn.test(@password, [], ZXCVBN_DICTIONNARIES) self.class.tester.test(@password)
end end
end end

View file

@ -1 +0,0 @@
ZXCVBN_DICTIONNARIES = YAML.safe_load(File.read(Rails.root.join("config", "initializers", "zxcvbn_dictionnaries.yaml")))

View file

@ -0,0 +1,47 @@
describe ZxcvbnService do
let(:password) { 'medium-strength-password' }
subject(:service) { ZxcvbnService.new(password) }
describe '#score' do
it 'returns the password complexity score' do
expect(service.score).to eq 3
end
end
describe '#complexity' do
it 'returns the password score, vulnerability and length' do
expect(service.complexity).to eq [3, 'medium, strength, password', 24]
end
end
describe 'caching' do
it 'lazily caches the tester between calls and instances' do
allow(Zxcvbn::Tester).to receive(:new).and_call_original
allow(YAML).to receive(:safe_load).and_call_original
first_service = ZxcvbnService.new('some-password')
first_service.score
first_service.complexity
other_service = ZxcvbnService.new('other-password')
other_service.score
other_service.complexity
expect(Zxcvbn::Tester).to have_received(:new).at_most(:once)
expect(YAML).to have_received(:safe_load).at_most(:once)
end
it 'lazily caches the tester between threads' do
allow(Zxcvbn::Tester).to receive(:new).and_call_original
threads = 1.upto(4).map do
Thread.new do
ZxcvbnService.new(password).score
end
end.map(&:join)
scores = threads.map(&:value)
expect(scores).to eq([3, 3, 3, 3])
expect(Zxcvbn::Tester).to have_received(:new).at_most(:once)
end
end
end