dtml-in batching accesses the complete list

[d-maurer]

DocumentTemplate 4.0

"zopefoundation/Products.ZCatalog#155" reports that Products.ZCatalog's manage_catalogView takes excessive times for large catalogs.
"zopefoundation/Products.ZCatalog#155 (comment)" contains corresponding profile results. They suggest that dtml-in accesses each element in the list even when called with batching parameters.

[d-maurer]

The behavior is caused by the list conversion in "

DocumentTemplate/src/DocumentTemplate/DT_In.py

Line 478 in 9496ce6

sequence = list(sequence)

".
It is there to support iterators in addition to true sequences but of course it destroys lazy access to a sequence (highly wished for batching).

I do not yet know how to distinguish safely between a true sequence (not requiring a list conversion) and something requiring a list conversion (e.g. a general iterator).

[d-maurer]

@sauzher found out that the list conversion has been introduced by @hannosch in "5716eb8". Unfortunately, the prevents lazy batching.

[sauzher]

For what I see, there are a lot of places where sequence gets unpacked: to-list conversions and key-lookup access:

List conversions:

• DocumentTemplate/src/DocumentTemplate/DT_InSV.py

  Line 41 in 9496ce6

  items = list(items)

• DocumentTemplate/src/DocumentTemplate/DT_In.py

  Line 478 in 9496ce6

  sequence = list(sequence)

Key lookups:

• DocumentTemplate/src/DocumentTemplate/DT_InSV.py

  Line 413 in 9496ce6

  sequence[start - 1]

• DocumentTemplate/src/DocumentTemplate/DT_InSV.py

  Line 423 in 9496ce6

  sequence[end + orphan - 1]

• DocumentTemplate/src/DocumentTemplate/DT_InSV.py

  Line 429 in 9496ce6

  sequence[end - 1]

• DocumentTemplate/src/DocumentTemplate/DT_InSV.py

  Line 440 in 9496ce6

  sequence[end + orphan - 1]

Key lookups I think are unnecessary because all the try-execeps blocks could be rewritten as simple if-then-else checks on start/end values.

List conversions perhaps could be refactored to convert in list only the firsts "batch_size * Page number" elements.

my 2 cent

[d-maurer]

I found out that dtml-in (as it is now) does not support arbitrary iterators:

DocumentTemplate/src/DocumentTemplate/DT_In.py

Line 468 in 9496ce6

if not sequence:

uses not sequence and this will be False for general iterators even if the iterator does not give any values. The list conversion comes too late to remedy this.

[d-maurer]

The original implementation tried hard to avoid the computation of the sequence length (by e.g. using try: sequence[end] except: to check whether there is an element at end) because it wanted support for lazy batching and for some lazy sequences length computation is expensive. Of course, converting the sequence into a list destroys this aim (at it computes the length as a side effect).

I propose to retain the support for lazy batching as follows:
We check whether the sequence has __len__ and __getitem__ but lacks keys; this first indicates that it may be designed as a Sequence, the second that it likely is not designed as a Mapping. We use the sequence as is if the check succeeds; otherwise we replace it by its list conversion.
@hannosch What do you think?

[d-maurer]

Alessandro Ceglie wrote at 2024-9-27 03:12 -0700:

For what I see, there are a lot of places where sequence gets unpacked: to-list conversions and key-lookup access: List conversions: - https://github.com/zopefoundation/DocumentTemplate/blob/9496ce63234a849961ef7bcd85890632bb14bf3d/src/DocumentTemplate/DT_InSV.py#L41 - https://github.com/zopefoundation/DocumentTemplate/blob/9496ce63234a849961ef7bcd85890632bb14bf3d/src/DocumentTemplate/DT_In.py#L478

I propose the following approach: We check (using a heuristics) whether the original sequence supports "key access" and if it does we use it directly. If it does not we use its list conversion. Unfortunately, checking for "key access" is not trivial. What we actually want to check for is sequence subscription, i.e. we should look for `__getitem__`; but `__getitem__` is used for mapping subscription, too. Therefore, I tend to the following heuristics: If the original sequence has `__len__` and `__getitem__` (potentially a sequence) but is not an instance of `collections.abs.Mapping` and has neither `keys` nor `get` (likely not a `Mapping`), then we use it directly. An alternativ to a list conversion could be the use of a lazy `iter2list` wrapper. It would ensure "key access", by iteration and caching of the obtained values.

Key lookups:

If we have guaranteed that the used sequence does support "key lookup", they no longer provide problems.