[issue37101] Filterer.filter can be rewritten using built-ins just as efficient much more readable
New submission from Dale Visser : Alternative version of Filterer.filter(...) would look like this, which takes advantage of efficient Python 3.x built-ins, and is immediately understandable: def _filter_callable(filter): return filter.filter if hasattr(filter, 'filter') else filter def filter(self, record): filters = map(_filter_callable, self.filters) return all(f(record) for f in filters) I will add a tested pull request on GitHub. -- components: Library (Lib) messages: 343983 nosy: dwvisser priority: normal severity: normal status: open title: Filterer.filter can be rewritten using built-ins just as efficient much more readable type: enhancement versions: Python 3.9 ___ Python tracker <https://bugs.python.org/issue37101> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37101] Filterer.filter can be rewritten using built-ins just as efficient much more readable
Change by Dale Visser : -- keywords: +patch pull_requests: +13570 stage: -> patch review pull_request: https://github.com/python/cpython/pull/13683 ___ Python tracker <https://bugs.python.org/issue37101> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37101] Filterer.filter can be rewritten using built-ins just as efficient much more readable
Dale Visser added the comment: Great! My PR now uses getattr as @zach.ware has suggested. -- ___ Python tracker <https://bugs.python.org/issue37101> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37101] Filterer.filter can be rewritten using built-ins just as efficient much more readable
Dale Visser added the comment: It is with great surprise and sadness that I report that I found that performance measurements don't support merging this change. I'm attaching my performance test script. I ran it with/without the logging filters for 100 iterations, e.g., ./python all 100 The results on my system were as follows, where the numbers may be interpreted as milliseconds per 1000 log messages: No Filters3 Filters master17.9 19.8 CBO-37101 19.6 23.1 I imagine I could put a guard 'if' statement that would restore the 'no filters' performance to match 'master', but my use of generators apparently causes the use of significantly more cycles than the existing code. -- resolution: -> rejected stage: patch review -> resolved status: open -> closed Added file: https://bugs.python.org/file48381/test_filter_perf.py ___ Python tracker <https://bugs.python.org/issue37101> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37101] Filterer.filter can be rewritten using built-ins just as efficient much more readable
Dale Visser added the comment: Correction on that example of running the test script: ./python test_filter_perf.py all 100 I simply prepended this with the Linux `time` command for measuring. -- ___ Python tracker <https://bugs.python.org/issue37101> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37101] Filterer.filter can be rewritten using built-ins just as efficient much more readable
Dale Visser added the comment: Adding the patch file associated with GitHub pull request #13683, as well as the performance test I just reported. -- Added file: https://bugs.python.org/file48383/pr13683.diff ___ Python tracker <https://bugs.python.org/issue37101> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37101] Filterer.filter can be rewritten using built-ins just as efficient much more readable
Dale Visser added the comment: FWIW, when I tried this instead, I got 21 to 22 msec per 1000 log messages (still a 5-10% performance hit): def filter(self, record): rv = True if self.filters: rv = all(getattr(f, 'filter', f)(record) for f in self.filters) return rv I don't see that getattr(...) should be less efficient than hasattr(...), so the generator expression is the likely culprit. -- ___ Python tracker <https://bugs.python.org/issue37101> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37101] Filterer.filter can be rewritten using built-ins just as efficient much more readable
Dale Visser added the comment: I've learned a lot about the performance trade-offs of generator expressions. The only way of shortening this code (readability is subjective) that I've found not to negatively impact performance is this: def filter(self, record): rv = True for f in self.filters: if not getattr(f, 'filter', f)(record): rv = False break return rv -- ___ Python tracker <https://bugs.python.org/issue37101> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
