Skip to content
New issue

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

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

Already on GitHub? # to your account

Make minor performance improvements #470

Merged

Conversation

dcslagel
Copy link
Collaborator

@dcslagel dcslagel commented Jun 5, 2021

Description:

This pull-request is an attempt to improve performance:

  • Reduce regex compiles by changing them to or with |
  • Move the import of OpenPyx from init.py to las.py::to_excel(). This saves about .038 seconds on Lasio loading.
  • Move the split_on_whitespace regex so that it is compiled only once and called directly when needed.

Performance Results:

--------------------------------------------------- benchmark: 1 tests ---------------------------------
Name (time in ms)               Min       Max      Mean  StdDev    Median     IQR  Outliers    OPS  Rounds  
--------------------------------------------------------------------------------------------------------
test_read_v12_sample_big     891.9103  901.8956  895.0018  4.0155  893.1709  4.0403       1;0  1.1173   5
--------------------------------------------------------------------------------------------------------

This is a small amount faster than previous performance, which was (roughly):

test_read_v12_sample_big     944.9445  957.4294  950.0452

Test Coverage:

Name                       Stmts   Miss  Cover
----------------------------------------------
lasio/__init__.py              8      0   100%
lasio/convert_version.py      20     20     0%
lasio/d1.py                   11     11     0%
lasio/defaults.py             11      0   100%
lasio/examples.py             42     10    76%
lasio/excel.py                88     34    61%
lasio/exceptions.py            6      0   100%
lasio/las.py                 443     62    86%
lasio/las_items.py           199     29    85%
lasio/las_version.py          50     14    72%
lasio/reader.py              432     27    94%
lasio/writer.py              171      9    95%
----------------------------------------------
TOTAL                       1481    216    85%

--
Let me know if this change could be accepted (or rejected) or
needs some additional changes to be approved and merged.

Thank you,
DC

@dcslagel dcslagel requested a review from kinverarity1 June 5, 2021 00:12
@dcslagel dcslagel marked this pull request as draft June 11, 2021 21:35
Copy link
Owner

@kinverarity1 kinverarity1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great thank you!

@kinverarity1
Copy link
Owner

If you think this is ready to merge I'm happy to

@dcslagel dcslagel marked this pull request as ready for review June 16, 2021 22:42
@dcslagel
Copy link
Collaborator Author

dcslagel commented Jun 16, 2021

Yes, this is ready go in.
Thanks!
DC

@kinverarity1 kinverarity1 merged commit 948f9b8 into kinverarity1:master Jun 17, 2021
@dcslagel dcslagel deleted the minor-performance-improvements branch June 17, 2021 16:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants