upgrade-util icon indicating copy to clipboard operation
upgrade-util copied to clipboard

[IMP] snippets: move all work from parent to mp workers

Open cawo-odoo opened this issue 1 year ago • 8 comments

In convert_html_columns(), we select 100MiB worth of DB tuples and pass them to a ProcessPoolExecutor together with a converter callable. So far, the converter returns all tuples, changed or unchanged together with the information if it has changed something. All this is returned through IPC to the parent process. In the parent process, the caller only acts on the changed tuples, though, the rest is ignored. In any scenario I've seen, only a small proportion of the input tuples is actually changed, meaning that a large proportion is returned through IPC unnecessarily.

What makes it worse is that processing of the converted results in the parent process is often slower than the conversion, leading to two effects:

  1. The results of all workers sit in the parent process's memory, possibly leading to MemoryError (upg-2021031)
  2. The parallel processing is being serialized on the feedback, defeating a large part of the intended performance gains

To improve this, this commit

  • moves all work into the workers, meaning not just the conversion filter, but also the DB query as well as the DB updates.
    • by doing so reduces the amount of data passed by IPC to just the query texts
    • by doing so distributes the data held in memory to all worker processes
  • reduces the chunk size by one order of magnitude, which means
    • a lot less memory used at a time
    • a lot better distribution of "to-be-changed" rows when these rows are clustered in id-space of the table

All in all, in my test case, this

  • reduces maximum process size in memory to 300MiB for all processes compared to formerly >2GiB (and MemoryError) in the parent process
  • reduces runtime from 17 minutes to less than 2 minutes

cawo-odoo avatar Sep 13 '24 14:09 cawo-odoo

Pull request status dashboard

robodoo avatar Sep 13 '24 14:09 robodoo

ok, I stopped the mattness. If anyone wants to review ... :-)

cawo-odoo avatar Sep 16 '24 12:09 cawo-odoo

all comments resolved

cawo-odoo avatar Sep 23 '24 11:09 cawo-odoo

𓃠 ?

cawo-odoo avatar Oct 09 '24 06:10 cawo-odoo

🐈 ?

cawo-odoo avatar Nov 13 '24 13:11 cawo-odoo

@nseinlet concerning our earlier coffee-machine meeting: wdyt about the fixup just pushed?

cawo-odoo avatar Nov 14 '24 14:11 cawo-odoo

rebased, conflicts resolved

cawo-odoo avatar May 15 '25 09:05 cawo-odoo

@nseinlet concerning our earlier coffee-machine meeting: wdyt about the fixup just pushed?

Today I improved the comments about the backwards compatibility, rebased, squashed the fixups. Do I get a review? And/or a :black_cat: ?

cawo-odoo avatar Jun 12 '25 07:06 cawo-odoo