Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Large performance overhead due to logging #20

Closed
kmnhan opened this issue Oct 31, 2024 · 1 comment · Fixed by #21
Closed

Large performance overhead due to logging #20

kmnhan opened this issue Oct 31, 2024 · 1 comment · Fixed by #21

Comments

@kmnhan
Copy link

kmnhan commented Oct 31, 2024

While profiling, I noticed that most of the overhead in loading files actually comes from the pprint module used in logging. The following test demonstrates this by comparing the time it takes to load test files with and without logging.

import igor2.binarywave
import igor2.packed
import logging

igor2_test_data_dir = "path/to/igor2/tests/data/"

ibw_file = igor2_test_data_dir + "mac-double.ibw"
pxp_file = igor2_test_data_dir + "polar-graphs-demo.pxp"

# ibw, logging enabled
logging.getLogger("igor2.struct").setLevel(logging.NOTSET)
%timeit igor2.binarywave.load(ibw_file)
# 6.81 ms ± 111 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

# ibw, logging disabled
logging.getLogger("igor2.struct").setLevel(logging.CRITICAL)
%timeit igor2.binarywave.load(ibw_file)
# 320 µs ± 5.69 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)

# pxp, logging enabled
logging.getLogger("igor2.struct").setLevel(logging.NOTSET)
%timeit igor2.packed.load(pxp_file)
# 249 ms ± 2.23 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

# pxp, logging disabled
logging.getLogger("igor2.struct").setLevel(logging.CRITICAL)
%timeit igor2.packed.load(pxp_file)
# 9.24 ms ± 65.7 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

The time it takes to load the files without logging is significantly faster than with logging. Would it be possible to disable logging by default?

@paulmueller
Copy link
Member

Interesting, there are a lot of debug statements in that module. I guess logging.WARNING or logging.INFO would also be sufficient in this case. I am happy to merge a PR with a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants