[IMP] core: remove useless mapped call when calling filtered with a str

[ivantodorovich]

The mapped call is supposed to populate the cache, but that's already taking care of by prefetching.
It ends up adding an overhead instead.

The perf improvement is more noticeable on large recordsets.
For example, on a database populated with 100k res.partner records.

Before:

    partners = env["res.partner"].search([])
    partners.filtered("name")  # warm up
    timeit.timeit(lambda: partners.filtered("name"), number=100)
    # result: 71.54

After:

    partners = env["res.partner"].search([])
    partners.filtered("name")  # warm up
    timeit.timeit(lambda: partners.filtered("name"), number=100)
    # result: 46.72

Although the perf imp is miniscule in real world cases, these lines can be misleading IMO.

---

I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

[robodoo]

[rco-odoo]

Thanks for the proposal, but we need some deeper analysis.

I mean that your analysis in the description above is flawed. The code that you remove is responsible for "warming up" the cache efficiently. If the cache is already "warm", the code is by definition useless, and removing it obviously makes filtered faster. We have to check that removing this code does not harm the performance of filtered when the cache is cold.

@ryv-odoo is going to check this.

[ivantodorovich]

Thank you both,

In my example I wanted to show that mapped has a cost, even when the cache is warm.
Here's the same comparisson when the cache is cold:

Before:

    partners = env["res.partner"].search([])
    timeit.timeit(lambda: partners.filtered("name") and partners.env.cache.clear(), number=100)
    # result: 495.05

After:

    partners = env["res.partner"].search([])
    timeit.timeit(lambda: partners.filtered("name") and partners.env.cache.clear(), number=100)
    # result: 281.33

My reasoning is that the cache doesn't need warming up. There's no real gain in perf, because otherwise the prefetching will just work, and do a good job at it.

[ryv-odoo]

Hello @ivantodorovich , Thank you

It seems very interesting but your numbers seems over optimistic. What is your computer set up (python version, etc) ?

On my computer:

• Before:
  
• After (with your change):
  

(2700 partners)

[ryv-odoo]

My previous comment is not complete and I found why we don't have the same result.

There is complexity issue with mapped (in fields.py). It isn't linear (where it should be): It is actually O(n * n/1000) (1000 = PREFETCH_MAX). Then when the number of record increase, the mapped costs becomes noticeable (how much partner do you have in your example ?).

In my opinion we should fix mapped first to see if you change is still pertinent. I am on it :)

[ivantodorovich]

Hello @ryv-odoo ,

I'm running these snippets on a db with roughly 100k partners. So yeah, plenty more than yours

if there's something to improve in mapped, it will definitely have a bigger impact. But anwyay, I still think this mapped call on filtered is pointless, don't you? The cache needs no warming up, prefetching will do it's work as the records are being filtered

[ryv-odoo]

Hello,

The cache needs no warming up, prefetching will do it's work as the records are being filtered

Yes it is correct. The mapped of Field class is optimize to reduce the number of browse record created (until the quadratics issue become noticeable of course), not to fill the cache more efficiently. Then this extra mapped is useless.

I verified with others parameters/models (filtered with complex relational path by example) to ensure that it is always more efficient. And the conclusion is the same than yours.

@robodoo r+

Thank you @ivantodorovich

The mapped issue will be addressed later.