-
Notifications
You must be signed in to change notification settings - Fork 60
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
Performance improvement 904 #907
Conversation
… PerformanceImprovement_904 # Conflicts: # forte/data/data_pack.py
…compatibility test)
…ommenting out related code for handling empty sets.
Codecov Report
@@ Coverage Diff @@
## master #907 +/- ##
=======================================
Coverage 81.00% 81.01%
=======================================
Files 254 254
Lines 19583 19583
=======================================
+ Hits 15864 15865 +1
+ Misses 3719 3718 -1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR is simple and I think it is fine.
Just fix the first line of the PR message, use the issue url.
Sorry the issue link previously was using a wrong URL of my branch. Just changed to the public issue URL of 904. |
I just updated the regular way. |
This PR fixes #904.
Description of changes
Profiling test find bottleneck in the "get"method of Entry in data_pack.py where using len() causing
slowdown so after discussion this is now fixed using underlying number count.
Possible influences of this PR.
This change can significantly improve the performance and the impact is minimal .
Test Conducted
This should pass all existing unit test code and have good performance on the profiling test.