Micro-optimisation for functional.keep_lazy for single argument uses.

[Keryn Knight]

keep_lazy has an internal decorator function, wrapper which may get called a lot during template rendering, because it's ultimately used by conditional_escape which is in turn used by render_value_into_context.

Rendering the standard admin change form for a user via client.get(f"/auth/user/1/change/") 100 times gives me the following cprofile output:

   11694527 function calls (11041473 primitive calls) in 8.609 seconds
   Ordered by: internal time
   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
30200/300    0.307    0.000    6.703    0.022 defaulttags.py:160(render)
     6434    0.242    0.000    0.250    0.000 {built-in method io.open}
   174600    0.218    0.000    0.682    0.000 base.py:849(_resolve_lookup)
   194000    0.204    0.000    1.120    0.000 base.py:698(resolve)
29200/500    0.175    0.000    6.720    0.013 loader_tags.py:168(render)
    30302    0.162    0.000    0.735    0.000 base.py:654(__init__)
    34240    0.161    0.000    0.355    0.000 base.py:779(__init__)
15137/5509    0.157    0.000    1.555    0.000 base.py:455(parse)
    91639    0.144    0.000    0.542    0.000 functional.py:226(wrapper).  # 1.6%?
...

with wrapper being the important line, and then:

In [1]: from django.utils.html import escape
In [2]: %timeit escape('<abc>d&g</abc>')
2.2 µs ± 51.3 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

This is because the current implementation is:

if any(isinstance(arg, Promise) for arg in itertools.chain(args, kwargs.values())):
    return lazy_func(*args, **kwargs)
return func(*args, **kwargs)

that is, it's optimised for the general case of N arguments. But Django's internal usage of keep_lazy is nearly unanimously on functions with a single argument.

It is possible to derive (at decorator time rather than call time) whether or not the function being wrapped needs to support multiple arguments, via inspect.signature and dispatch to a different wrapper, which offers somewhat better performance.

A super naive implementation looks like:

@wraps(func)
def keep_lazy_single_argument_wrapper(arg):
    if isinstance(arg, Promise):
        return lazy_func(arg)
    return func(arg)

@wraps(func)
def keep_lazy_multiple_argument_wrapper(*args, **kwargs):
    if any(isinstance(arg, Promise) for arg in itertools.chain(args, kwargs.values())):
        return lazy_func(*args, **kwargs)
    return func(*args, **kwargs)

if len(inspect.signature(func).parameters) == 1:
    return keep_lazy_single_argument_wrapper
else:
    return keep_lazy_multiple_argument_wrapper

which provides the best difference:

11327832 function calls (10674778 primitive calls) in 9.062 seconds
   Ordered by: internal time
   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
...
    91639    0.059    0.000    0.339    0.000 functional.py:227(keep_lazy_single_argument_wrapper). # 0.6% of time

and the actual usage time:

In [2]: %timeit escape('<abc>d&g</abc>')
1.5 µs ± 16.5 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

Though it comes at the cost of no longer being able to use keyword arguments:

In [3]: escape(text=1)
TypeError: keep_lazy_single_argument_wrapper() got an unexpected keyword argument 'text'

which can be worked around by doing something (back of the napkin) like:

func_params = inspect.signature(func).parameters
func_first_param_name = tuple(func_params.keys())[0]

@wraps(func)
def keep_lazy_single_argument_wrapper(*args, **kwargs):
    if (args and isinstance(args[0], Promise)) or ('func_first_param_name' in kwargs and isinstance(kwargs.get(func_first_param_name), Promise)):
        return lazy_func(*args, **kwargs)
    return func(*args, **kwargs)
...

which still seems to be better:

   11327896 function calls (10674842 primitive calls) in 9.411 seconds
   Ordered by: internal time
   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
...
    91639    0.088    0.000    0.382    0.000 functional.py:230(keep_lazy_single_argument_wrapper). # 0.9%

and:

In [2]: %timeit escape('<abc>d&g</abc>')
1.64 µs ± 32.8 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [3]: escape(text=1)
'1'

and correctly works with invalid kwargs (this is as much a note to myself as anything, my previous attempts did not :)):

In [4]: escape(test=1)
TypeError: escape() got an unexpected keyword argument 'test'

[Keryn Knight]

PR is here: ​https://github.com/django/django/pull/14849
Currently waiting to see if a new revised version works, because it's too hot here at the moment, and there's a couple of problems with the variant proposed upthread. Most notably, checking for 'func_first_param_name' in kwargs like a fool, but also additionally handling funcs which themselves declare *args or **kwargs.

Performance remains roughly the same for single argument functions, unless they're using the kwarg form, in which case it's something like:

In [3]: %timeit escape(text='<abc>d&g</abc>')
1.89 µs ± 73.6 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

Ain't done any tests for it yet, because I shudder at the thought, knowing that they'll probably require using mock, and there's definitely little point doing them if the ticket doesn't have merit :)

[Carlton Gibson]

OK, looks reasonable — with suitable tests[*], and comments. Thanks Keryn.

[*]: "...being able to use keyword arguments..." — this is documented API, so we need to not change the behaviour (unless that argument is made separately...)

[Keryn Knight]

Let's go ahead and mark Needs Tests then, because I sure as heck haven't added any :)

[Keryn Knight]

I've added tests, but they're not _good_ ones, really. Hence this remains on patch needs improvement.

Specifically the tests demonstrate that re-wrapping with a different specialised wrapper doesn't cause the exceptions to _change_ when called incorrectly, and covers calling it in a bunch of different ways. But the tests _don't_ demonstrate which wrapper they passed through, or that they correctly use the lazy_func. I'd need some guidance on how to approach testing those aspects.

Linters are currently preventing it from being marked as successful (because of course they are), but the tests pass, such as they are.