35% better performance on scripts/benchmark_big_table.py

[bmritz]

Hi @pelme, I know this is unsolicited, but I thought I'd put it out there for you to take a look.

I noticed in some benchmarking that the isinstance(x, str | _HasHtml) line was taking up a lot of the runtime on the benchmark. I replaced it with a hasattr call (to check for the __html__ attribute), which turned out to be faster.

There's definitely a tradeoff here, because now we've got this awkward hasattr in the if, elif, elif checks, whereas all the others are checking isinstance. I also am not 100% sure here on all of the ramifications of checking for the __html__ attribute.

I'll leave it up to you if you think the optimization here is worth it. I don't have a great feel for the usage of this library to understand how important this codepath is, or how much this would help in real world usage...just thought I'd put it out there for your look.

Thanks for making a great abstraction for html in python!

Benchmarks

Prior to change

htpy: 0.1833320000441745 seconds - /var/folders/cy/lk5jk1h92d74hj_hn37mfxdr0000gp/T/tmpgd58t6c9/htpy_table.html
django: 0.26962741604074836 seconds - /var/folders/cy/lk5jk1h92d74hj_hn37mfxdr0000gp/T/tmpgd58t6c9/django_table.html
jinja2: 0.008415041025727987 seconds - /var/folders/cy/lk5jk1h92d74hj_hn37mfxdr0000gp/T/tmpgd58t6c9/jinja2_table.html

After Change

(htpy) ➜  htpy git:(performance-enhancements) ✗ python scripts/benchmark_big_table.py
htpy: 0.11880300007760525 seconds - /var/folders/cy/lk5jk1h92d74hj_hn37mfxdr0000gp/T/tmp5w2cwfx1/htpy_table.html
django: 0.2763869169866666 seconds - /var/folders/cy/lk5jk1h92d74hj_hn37mfxdr0000gp/T/tmp5w2cwfx1/django_table.html
jinja2: 0.007596875075250864 seconds - /var/folders/cy/lk5jk1h92d74hj_hn37mfxdr0000gp/T/tmp5w2cwfx1/jinja2_table.html

Original Results from Line Profiler:

Total time: 0.998289 s
File: /Users/britz/Downloads/htpy.py
Function: _iter_node_context at line 173

Line #      Hits         Time  Per Hit   % Time  Line Contents
==============================================================
   173                                           @line_profiler.profile
   174                                           def _iter_node_context(x: Node, context_dict: dict[Context[t.Any], t.Any]) -> Iterator[str]:
   175    150007   27365000.0    182.4      2.7      while not isinstance(x, BaseElement) and callable(x):
   176                                                   x = x()
   177                                           
   178    150007   15999000.0    106.7      1.6      if x is None:
   179                                                   return
   180                                           
   181    150007   16046000.0    107.0      1.6      if x is True:
   182                                                   return
   183                                           
   184    150007   14056000.0     93.7      1.4      if x is False:
   185                                                   return
   186                                           
   187    150007   21740000.0    144.9      2.2      if isinstance(x, BaseElement):
   188    100004  180995000.0   1809.9     18.1          yield from x._iter_context(context_dict)  # pyright: ignore [reportPrivateUsage]
   189     50003    8858000.0    177.1      0.9      elif isinstance(x, ContextProvider):
   190                                                   yield from _iter_node_context(x.func(), {**context_dict, x.context: x.value})  # pyright: ignore [reportUnknownMemberType]
   191     50003    8245000.0    164.9      0.8      elif isinstance(x, ContextConsumer):
   192                                                   context_value = context_dict.get(x.context, x.context.default)
   193                                                   if context_value is _NO_DEFAULT:
   194                                                       raise LookupError(
   195                                                           f'Context value for "{x.context.name}" does not exist, '
   196                                                           f"requested by {x.debug_name}()."
   197                                                       )
   198                                                   yield from _iter_node_context(x.func(context_value), context_dict)
   199     50003  339072000.0   6781.0     34.0      elif isinstance(x, str | _HasHtml):
   200         1       6000.0   6000.0      0.0          yield str(_escape(x))
   201     50002    8190000.0    163.8      0.8      elif isinstance(x, int):
   202     50000   26023000.0    520.5      2.6          yield str(x)
   203         2       7000.0   3500.0      0.0      elif isinstance(x, Iterable) and not isinstance(x, _KnownInvalidChildren):  # pyright: ignore [reportUnnecessaryIsInstance]
   204     50004  296610000.0   5931.7     29.7          for child in x:
   205     50002   35077000.0    701.5      3.5              yield from _iter_node_context(child, context_dict)
   206                                               else:
   207                                                   raise TypeError(f"{x!r} is not a valid child element")

[bmritz]

I need to do a little more homework on this... I'm not seeing the performance improvements I thought I saw. I must have had something different between the two tests.

[pelme]

thanks for looking into this! let me know if you find any useful optimizations!