[13.0][OU-FIX] fix product images memory error

[huguesdk]

avoid MemoryError when converting product images (in product and website_sale).

fixes #4712.

[pedrobaeza]

I think this impacts in performance, isn't it?

[huguesdk]

maybe, i didn’t profile this. per image, it will make one database query to get the image, then several ones when creating the new attachments, but since reading, resizing and writing image attachments is way slower than doing some queries, i think the impact should be minimal.

[pedrobaeza]

Not sure... Maybe this can be optional through a system param or similar?

[pedrobaeza]

Or do it by batches of a size like 500?

[huguesdk]

iirc, i think the main issue comes from the fact that the smaller-size images are computed when flushing the model. i see in invalidate_cache() (called by chunked()), that only the base model is flushed instead of the one we’re working with. could it be why chunked() wasn’t working in this case? chunked() loads PREFETCH_MAX (which is 1000) records at the time. with images this can quickly add up, although in most cases that would still need under a few gib of memory (but odoo’s default hard limit is 2560 mib).

if we’re doing it in batches of 500 for performance, then it would make sense to also prefetch the records by the same batch size. unfortunately, i don’t have time currently to test this, but feel free to suggest something if you have.

[MiquelRForgeFlow]

I think the batches can be of size 1000.

[huguesdk]

i’ve just made a test with this code unchanged and again after changing it like this (in the same way as openupgrade.chunked()) (CHUNK_SIZE being 512 here):

        for i in range(0, len(attachment_ids), CHUNK_SIZE):
            # load attachments chunk by chunk to avoid prefetching too many
            for attachment in attachment_model.browse(
                attachment_ids[i : i + CHUNK_SIZE]
            ):
                try:
                    Model.browse(
                        attachment.res_id
                    ).image_1920 = attachment.datas
                except Exception as e:
                    _logger.error(
                        "Error while recovering %s>%s for %s: %s",
                        model,
                        field,
                        attachment.res_id,
                        repr(e),
                    )
            # flush and clear cache to avoid to fill up memory and force the
            # computation of the computed fields (smaller size images)
            Model.flush()
            env.cache.invalidate()

with 24493 images (with a total size of 25 gib), it took 02:15:29 (2 hours 15 minutes) with the code unchanged and 02:17:22 (2 hours 17 minutes) with the change shown here. so it is actually (slightly) faster doing it one by one. i think that there is so much time spent processing images that database access time is not significant here.

[pedrobaeza]

And do you have the times without both changes (all together)?

[huguesdk]

without any change, it crashes with a MemoryError.

[pedrobaeza]

Yeah, but we are trying to set if putting this for everyone (crashing or not) is going to affect performance. Maybe you should do the test in a smaller one, and compare times.