graphite-project / graphite-web

A highly scalable real-time graphing system
http://graphite.readthedocs.org/
Apache License 2.0
5.89k stars 1.26k forks source link

len() was used as condition fixed for performance #2721

Closed Spratiher9 closed 2 years ago

Spratiher9 commented 2 years ago

In line 245 len() was used as condition. Using the len function to check if a sequence is empty is not idiomatic and can be less performant than checking the truthiness of the object. So removed it

DanCech commented 2 years ago

Does this actually make any difference? I ask because series isn't a bare list but rather a custom object that extends list, which is likely why the explicit len is being used here (to remove any doubt about the intent).

Spratiher9 commented 2 years ago

@DanCech yes this one small thing is potentially a big performance anti-pattern The idea is to check whether it is containing any value or is it something that is synonymous with False or None or Blank or empty string or 0. Len() running this computation unnecessarily is not making sense.

DanCech commented 2 years ago

What I mean is have you done any performance testing on the change?

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.