Avoid Django's lazy() when creating validators because it is too slow

As a fix for issue #3354, commit 607e4edca7 made the
evaluation of some validation error messages lazy. To achieve that,
Django's django.utils.functional.lazy() function was used. However, that
function is extremely heavy and slow, and slows down string validation
significantly (lazy() is evaluated each time for each validator for each
field which has one). We noticed this in our production system.

Avoid lazy and use a lambda to defer the evaluation instead. This uses
the fact that a Django validator is just a simple callable.

Using the benchmark attached to the PR (snipped to tottime>100ms):

Before, model serializer:

         9225123 function calls (9200068 primitive calls) in 8.337 seconds

   Ordered by: internal time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
    25000    1.299    0.000    3.534    0.000 functional.py:125(__prepare_class__)
  2415001    0.954    0.000    0.954    0.000 {built-in method builtins.hasattr}
  1350000    0.901    0.000    0.901    0.000 functional.py:145(__promise__)
  1550001    0.521    0.000    0.521    0.000 {built-in method builtins.setattr}
    25000    0.494    0.000    0.735    0.000 {built-in method builtins.__build_class__}
    30000    0.298    0.000    0.385    0.000 fields.py:297(__init__)
    25000    0.289    0.000    5.895    0.000 fields.py:740(__init__)
    25000    0.264    0.000    0.802    0.000 field_mapping.py:66(get_field_kwargs)
    25000    0.241    0.000    0.241    0.000 functional.py:100(__proxy__)
670003/670001    0.203    0.000    0.211    0.000 {built-in method builtins.getattr}
     5000    0.189    0.000    7.722    0.002 serializers.py:990(get_fields)
    25000    0.186    0.000    0.400    0.000 functools.py:186(total_ordering)
    25000    0.158    0.000    0.299    0.000 functional.py:234(wrapper)
     5000    0.129    0.000    0.136    0.000 serializers.py:1066(get_field_names)
    25000    0.104    0.000    1.002    0.000 serializers.py:1195(build_standard_field)

After, model serializer:

         3400123 function calls (3400068 primitive calls) in 2.824 seconds

   Ordered by: internal time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
    30000    0.259    0.000    0.341    0.000 fields.py:294(__init__)
    25000    0.242    0.000    0.674    0.000 field_mapping.py:66(get_field_kwargs)
     5000    0.176    0.000    2.209    0.000 serializers.py:990(get_fields)
    25000    0.133    0.000    0.514    0.000 fields.py:737(__init__)
295003/295001    0.106    0.000    0.115    0.000 {built-in method builtins.getattr}

Before, regular serializer:

         8060003 function calls (7960003 primitive calls) in 7.123 seconds

   Ordered by: internal time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
    25000    1.569    0.000    3.897    0.000 functional.py:125(__prepare_class__)
  1350000    1.013    0.000    1.013    0.000 functional.py:145(__promise__)
  2365000    0.925    0.000    0.925    0.000 {built-in method builtins.hasattr}
  1550000    0.512    0.000    0.512    0.000 {built-in method builtins.setattr}
    25000    0.378    0.000    0.550    0.000 {built-in method builtins.__build_class__}
    25000    0.307    0.000    5.946    0.000 fields.py:740(__init__)
    30000    0.277    0.000    0.360    0.000 fields.py:297(__init__)
80000/5000    0.202    0.000    6.526    0.001 copy.py:132(deepcopy)
    25000    0.172    0.000    0.172    0.000 functional.py:100(__proxy__)
   540000    0.152    0.000    0.152    0.000 {built-in method builtins.getattr}
    25000    0.119    0.000    6.199    0.000 fields.py:604(__deepcopy__)

After, regular serializer:

         2010003 function calls (1935003 primitive calls) in 1.456 seconds

   Ordered by: internal time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
    30000    0.194    0.000    0.263    0.000 fields.py:294(__init__)
80000/5000    0.151    0.000    0.923    0.000 copy.py:132(deepcopy)
    25000    0.128    0.000    0.470    0.000 fields.py:737(__init__)
This commit is contained in:
Ran Benita 2019-05-02 18:46:02 +03:00
parent 1e519486e1
commit 99a2b7c347
3 changed files with 69 additions and 88 deletions

View File

@ -5,7 +5,6 @@ versions of Django/Python, and compatibility wrappers around optional packages.
import sys import sys
from django.conf import settings from django.conf import settings
from django.core import validators
from django.views.generic import View from django.views.generic import View
try: try:
@ -252,34 +251,5 @@ LONG_SEPARATORS = (', ', ': ')
INDENT_SEPARATORS = (',', ': ') INDENT_SEPARATORS = (',', ': ')
class CustomValidatorMessage:
"""
We need to avoid evaluation of `lazy` translated `message` in `django.core.validators.BaseValidator.__init__`.
https://github.com/django/django/blob/75ed5900321d170debef4ac452b8b3cf8a1c2384/django/core/validators.py#L297
Ref: https://github.com/encode/django-rest-framework/pull/5452
"""
def __init__(self, *args, **kwargs):
self.message = kwargs.pop('message', self.message)
super().__init__(*args, **kwargs)
class MinValueValidator(CustomValidatorMessage, validators.MinValueValidator):
pass
class MaxValueValidator(CustomValidatorMessage, validators.MaxValueValidator):
pass
class MinLengthValidator(CustomValidatorMessage, validators.MinLengthValidator):
pass
class MaxLengthValidator(CustomValidatorMessage, validators.MaxLengthValidator):
pass
# Version Constants. # Version Constants.
PY36 = sys.version_info >= (3, 6) PY36 = sys.version_info >= (3, 6)

View File

@ -12,7 +12,8 @@ from django.conf import settings
from django.core.exceptions import ObjectDoesNotExist from django.core.exceptions import ObjectDoesNotExist
from django.core.exceptions import ValidationError as DjangoValidationError from django.core.exceptions import ValidationError as DjangoValidationError
from django.core.validators import ( from django.core.validators import (
EmailValidator, RegexValidator, URLValidator, ip_address_validators EmailValidator, MaxLengthValidator, MaxValueValidator, MinLengthValidator,
MinValueValidator, RegexValidator, URLValidator, ip_address_validators
) )
from django.forms import FilePathField as DjangoFilePathField from django.forms import FilePathField as DjangoFilePathField
from django.forms import ImageField as DjangoImageField from django.forms import ImageField as DjangoImageField
@ -23,17 +24,13 @@ from django.utils.dateparse import (
from django.utils.duration import duration_string from django.utils.duration import duration_string
from django.utils.encoding import is_protected_type, smart_text from django.utils.encoding import is_protected_type, smart_text
from django.utils.formats import localize_input, sanitize_separators from django.utils.formats import localize_input, sanitize_separators
from django.utils.functional import lazy
from django.utils.ipv6 import clean_ipv6_address from django.utils.ipv6 import clean_ipv6_address
from django.utils.timezone import utc from django.utils.timezone import utc
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext_lazy as _
from pytz.exceptions import InvalidTimeError from pytz.exceptions import InvalidTimeError
from rest_framework import ISO_8601 from rest_framework import ISO_8601
from rest_framework.compat import ( from rest_framework.compat import ProhibitNullCharactersValidator
MaxLengthValidator, MaxValueValidator, MinLengthValidator,
MinValueValidator, ProhibitNullCharactersValidator
)
from rest_framework.exceptions import ErrorDetail, ValidationError from rest_framework.exceptions import ErrorDetail, ValidationError
from rest_framework.settings import api_settings from rest_framework.settings import api_settings
from rest_framework.utils import html, humanize_datetime, json, representation from rest_framework.utils import html, humanize_datetime, json, representation
@ -744,14 +741,15 @@ class CharField(Field):
self.min_length = kwargs.pop('min_length', None) self.min_length = kwargs.pop('min_length', None)
super().__init__(**kwargs) super().__init__(**kwargs)
if self.max_length is not None: if self.max_length is not None:
message = lazy(self.error_messages['max_length'].format, str)(max_length=self.max_length) self.validators.append(lambda value: MaxLengthValidator(
self.validators.append( self.max_length,
MaxLengthValidator(self.max_length, message=message)) message=self.error_messages['max_length'].format(max_length=self.max_length),
)(value))
if self.min_length is not None: if self.min_length is not None:
message = lazy( self.validators.append(lambda value: MinLengthValidator(
self.error_messages['min_length'].format, str)(min_length=self.min_length) self.min_length,
self.validators.append( message=self.error_messages['min_length'].format(min_length=self.min_length),
MinLengthValidator(self.min_length, message=message)) )(value))
# ProhibitNullCharactersValidator is None on Django < 2.0 # ProhibitNullCharactersValidator is None on Django < 2.0
if ProhibitNullCharactersValidator is not None: if ProhibitNullCharactersValidator is not None:
@ -910,15 +908,15 @@ class IntegerField(Field):
self.min_value = kwargs.pop('min_value', None) self.min_value = kwargs.pop('min_value', None)
super().__init__(**kwargs) super().__init__(**kwargs)
if self.max_value is not None: if self.max_value is not None:
message = lazy( self.validators.append(lambda value: MaxValueValidator(
self.error_messages['max_value'].format, str)(max_value=self.max_value) self.max_value,
self.validators.append( message=self.error_messages['max_value'].format(max_value=self.max_value),
MaxValueValidator(self.max_value, message=message)) )(value))
if self.min_value is not None: if self.min_value is not None:
message = lazy( self.validators.append(lambda value: MinValueValidator(
self.error_messages['min_value'].format, str)(min_value=self.min_value) self.min_value,
self.validators.append( message=self.error_messages['min_value'].format(min_value=self.min_value),
MinValueValidator(self.min_value, message=message)) )(value))
def to_internal_value(self, data): def to_internal_value(self, data):
if isinstance(data, str) and len(data) > self.MAX_STRING_LENGTH: if isinstance(data, str) and len(data) > self.MAX_STRING_LENGTH:
@ -948,17 +946,15 @@ class FloatField(Field):
self.min_value = kwargs.pop('min_value', None) self.min_value = kwargs.pop('min_value', None)
super().__init__(**kwargs) super().__init__(**kwargs)
if self.max_value is not None: if self.max_value is not None:
message = lazy( self.validators.append(lambda value: MaxValueValidator(
self.error_messages['max_value'].format, self.max_value,
str)(max_value=self.max_value) message=self.error_messages['max_value'].format(max_value=self.max_value),
self.validators.append( )(value))
MaxValueValidator(self.max_value, message=message))
if self.min_value is not None: if self.min_value is not None:
message = lazy( self.validators.append(lambda value: MinValueValidator(
self.error_messages['min_value'].format, self.min_value,
str)(min_value=self.min_value) message=self.error_messages['min_value'].format(min_value=self.min_value),
self.validators.append( )(value))
MinValueValidator(self.min_value, message=message))
def to_internal_value(self, data): def to_internal_value(self, data):
@ -1007,16 +1003,15 @@ class DecimalField(Field):
super().__init__(**kwargs) super().__init__(**kwargs)
if self.max_value is not None: if self.max_value is not None:
message = lazy( self.validators.append(lambda value: MaxValueValidator(
self.error_messages['max_value'].format, self.max_value,
str)(max_value=self.max_value) message=self.error_messages['max_value'].format(max_value=self.max_value),
self.validators.append( )(value))
MaxValueValidator(self.max_value, message=message))
if self.min_value is not None: if self.min_value is not None:
message = lazy( self.validators.append(lambda value: MinValueValidator(
self.error_messages['min_value'].format, str)(min_value=self.min_value) self.min_value,
self.validators.append( message=self.error_messages['min_value'].format(min_value=self.min_value),
MinValueValidator(self.min_value, message=message)) )(value))
if rounding is not None: if rounding is not None:
valid_roundings = [v for k, v in vars(decimal).items() if k.startswith('ROUND_')] valid_roundings = [v for k, v in vars(decimal).items() if k.startswith('ROUND_')]
@ -1352,17 +1347,15 @@ class DurationField(Field):
self.min_value = kwargs.pop('min_value', None) self.min_value = kwargs.pop('min_value', None)
super().__init__(**kwargs) super().__init__(**kwargs)
if self.max_value is not None: if self.max_value is not None:
message = lazy( self.validators.append(lambda value: MaxValueValidator(
self.error_messages['max_value'].format, self.max_value,
str)(max_value=self.max_value) message=self.error_messages['max_value'].format(max_value=self.max_value),
self.validators.append( )(value))
MaxValueValidator(self.max_value, message=message))
if self.min_value is not None: if self.min_value is not None:
message = lazy( self.validators.append(lambda value: MinValueValidator(
self.error_messages['min_value'].format, self.min_value,
str)(min_value=self.min_value) message=self.error_messages['min_value'].format(min_value=self.min_value),
self.validators.append( )(value))
MinValueValidator(self.min_value, message=message))
def to_internal_value(self, value): def to_internal_value(self, value):
if isinstance(value, datetime.timedelta): if isinstance(value, datetime.timedelta):
@ -1605,11 +1598,15 @@ class ListField(Field):
super().__init__(*args, **kwargs) super().__init__(*args, **kwargs)
self.child.bind(field_name='', parent=self) self.child.bind(field_name='', parent=self)
if self.max_length is not None: if self.max_length is not None:
message = self.error_messages['max_length'].format(max_length=self.max_length) self.validators.append(lambda value: MaxLengthValidator(
self.validators.append(MaxLengthValidator(self.max_length, message=message)) self.max_length,
message=self.error_messages['max_length'].format(max_length=self.max_length),
)(value))
if self.min_length is not None: if self.min_length is not None:
message = self.error_messages['min_length'].format(min_length=self.min_length) self.validators.append(lambda value: MinLengthValidator(
self.validators.append(MinLengthValidator(self.min_length, message=message)) self.min_length,
message=self.error_messages['min_length'].format(min_length=self.min_length),
)(value))
def get_value(self, dictionary): def get_value(self, dictionary):
if self.field_name not in dictionary: if self.field_name not in dictionary:
@ -1876,10 +1873,10 @@ class ModelField(Field):
max_length = kwargs.pop('max_length', None) max_length = kwargs.pop('max_length', None)
super().__init__(**kwargs) super().__init__(**kwargs)
if max_length is not None: if max_length is not None:
message = lazy( self.validators.append(lambda value: MaxLengthValidator(
self.error_messages['max_length'].format, str)(max_length=self.max_length) self.max_length,
self.validators.append( message=self.error_messages['max_length'].format(max_length=self.max_length),
MaxLengthValidator(self.max_length, message=message)) )(value))
def to_internal_value(self, data): def to_internal_value(self, data):
rel = self.model_field.remote_field rel = self.model_field.remote_field

View File

@ -95,6 +95,20 @@ def pytest_configure(config):
settings.STATIC_ROOT = os.path.join(os.path.dirname(rest_framework.__file__), 'static-root') settings.STATIC_ROOT = os.path.join(os.path.dirname(rest_framework.__file__), 'static-root')
settings.STATICFILES_STORAGE = 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage' settings.STATICFILES_STORAGE = 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage'
# Test that serializer fields can be created before django is set-up.
# This is not an ideal place for a test, but what can you do.
# See issue #3354.
from rest_framework.serializers import (
CharField, DecimalField, DurationField, FloatField, IntegerField,
ListField,
)
CharField(min_length=1, max_length=2)
IntegerField(min_value=1, max_value=2)
FloatField(min_value=1, max_value=2)
DecimalField(max_digits=10, decimal_places=1, min_value=1, max_value=2)
DurationField(min_value=1, max_value=2)
ListField(min_length=1, max_length=2)
django.setup() django.setup()
if config.getoption('--staticfiles'): if config.getoption('--staticfiles'):