Opened 3 years ago

Closed 3 years ago

#18163 closed Cleanup/optimization (fixed)

Use a faster password hasher in test_sqlite.py

Reported by: akaariai Owned by: nobody
Component: contrib.auth Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently Django uses django.contrib.auth.hashers.PBKDF2PasswordHasher as the highest priority PASSWORD_HASHERS choice. This hasher is somewhat slow by design. The slowness results in 10% overhead on my machine compared to use of MD5PasswordHasher. The test is ./runtests.py --settings=test_sqlite.py, result is ~670s vs ~610s. On other databases the gained performance advantage is lost in the noise.

This is related to ticket #18157, which is about documenting the use of different default hasher in testing as an optimisation.

I noticed that the tests error out if the PASSWORD_HASHERS do not contain all of the inbuilt hashers. Is this something that needs fixing?

Attachments (1)

ticket_18163.diff (827 bytes) - added by akaariai 3 years ago.

Download all attachments as: .zip

Change History (5)

Changed 3 years ago by akaariai

comment:1 Changed 3 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

I noticed that the tests error out if the PASSWORD_HASHERS do not contain all of the inbuilt hashers. Is this something that needs fixing?

I suggest adding @override_settings(PASSWORD_HASHERS=...) so the tests don't depend on the value in the settings. It's also more explicit.

Then you can include only the md5 hasher in the test_sqlite settings.

comment:2 Changed 3 years ago by akaariai

There were a lot of tests that did not pass with custom settings.PASSWORD_HASHERS. Almost all of them seem to be related to fixtures containing passwords in sha1 format. I think I nailed them all...

admin_views is 50s->30s due to this change. I believe the real reason is that the code updates the password on first use if it is not the preferred password already - and every use is first because the tests flush the DB between tests.

The whole test suite runs in around 500s now, pre-patch around 550s. So, the speedup isn't huge, but still something.

Pull request https://github.com/django/django/pull/28 tracks work done. One patch resets the cached password hashers on setting_changed, second patch overrides settings in the tests where needed, and the last patch is adding MD5 hasher to test_sqlite.py.

comment:3 Changed 3 years ago by aaugustin

  • Triage Stage changed from Accepted to Ready for checkin

LGTM.

comment:4 Changed 3 years ago by kmtracey

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.
Back to Top