Improve performance of looks_like_numpy_member()

[jacobtylerwalls]

Type of Changes

	Type
✓	🔨 Refactoring

Description

This method is the hottest path in astroid by number of primitive calls(!) Every name and attribute goes through it, multiplied by the number of numpy method names we compare against.

We already know whether we have a Name or Attribute when registering the transform, so avoiding re-checking the type here saves 32% of the calls to isinstance() when linting the entire astroid package.

Benchmark

for pylint astroid on my M1 mac running python 3.11.2, a savings of 0.4s out of 16.7s, or about 2.5% wall time:

main

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
 12602852    0.658    0.000    0.717    0.000 {built-in method builtins.isinstance}
  2056140    0.426    0.000    0.581    0.000 brain_numpy_utils.py:64(looks_like_numpy_member)

PR

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
  8491012    0.510    0.000    0.568    0.000 {built-in method builtins.isinstance}
  1534080    0.101    0.000    0.101    0.000 brain_numpy_utils.py:64(name_looks_like_numpy_member)
...
   522000    0.034    0.000    0.038    0.000 brain_numpy_utils.py:72(attribute_looks_like_numpy_member)

[codecov]

Codecov Report

Merging #2178 (196aaf3) into main (e1b577a) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2178      +/-   ##
==========================================
- Coverage   92.60%   92.60%   -0.01%     
==========================================
  Files          94       94              
  Lines       10800    10798       -2     
==========================================
- Hits        10001     9999       -2     
  Misses        799      799              

Flag	Coverage Δ
linux	92.35% <100.00%> (-0.01%)	⬇️
pypy	87.55% <100.00%> (+0.01%)	⬆️
windows	92.19% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
astroid/brain/brain_numpy_core_function_base.py	100.00% <100.00%> (ø)
astroid/brain/brain_numpy_core_multiarray.py	100.00% <100.00%> (ø)
astroid/brain/brain_numpy_core_numeric.py	100.00% <100.00%> (ø)
astroid/brain/brain_numpy_utils.py	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

[Pierre-Sassoulas]

I'm wondering if we should treat numpy this way at all ? Could we create a pylint-numpy plugin that would be the only one doing the check ?

[DanielNoord]

Funny, I have local branch that fixes this exact issue.
Another improvement could be not to create a brain per method, but just pass a set of member names and see if the attribute name is in it. There is no need to have a separate brain for every possible member.

Do agree with Pierre though that ideally this should be moved out of the base astroid package.

[Pierre-Sassoulas]

Not sure if you followed that @jacobtylerwalls but we created a proof of concept "pylint-tensorflow" project to see if brains could be moved (tensorflow is heavy). Right now it's not in working condition, but if loading brains make astroid load slow, extending such plugins -- once we're clear about how they would work exactly -- would be within reach. Right now the idea would be to make user install the plugin if they use tensorflow, numpy, etc... Like they already do for django for example.

[jacobtylerwalls]

Sounds good. Let's do both. Let's improve this code and then when it's extracted into a plugin it's also improved code over there.

Another improvement could be not to create a brain per method, but just pass a set of member names and see if the attribute name is in it. There is no need to have a separate brain for every possible member.

Thought of this also; shouldn't be hard to do this on the same branch.

[jacobtylerwalls]

There is no need to have a separate brain for every possible member.

Oh, whoops, I'm unvolunteering for this part, as it's hairier than I anticipated. The method names correspond to specific function transforms. I think I'm going to take the W here and merge what's here.

Thanks for the reviews!