-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
fix: speed up slow flow board using static methods #7330
Conversation
library/patient_tracker.inc.php
Outdated
@@ -32,9 +32,12 @@ | |||
use OpenEMR\Services\AppointmentService; | |||
use OpenEMR\Services\PatientTrackerService; | |||
|
|||
$apppointmentService = new AppointmentService(); | |||
$patientTrackerService = new PatientTrackerService(); |
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.
Are their specific queries/code that are slow in these classes during their initiation?
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.
Not seeing slow instantiation, bogs down from hundreds of unnecessary new objects.
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.
Seems like would be difficult to ensure these class instances can be shared like this. Three other options would be:
- Change the pertinent functions in the class to static (need to ensure the function don't do any $this stuff in them)
- Call the class/functions directly in patient_flow_board_report.php (rather than using the wrappers in patient_tracker.inc.php)
- Combination of 1 and 2
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.
Everytime the BaseService is instantiated it hits the database to grab all of the columns and metadata about the table. I've been annoyed for a while about this behavior. We either need to start switching to a single service Repository pattern to mitigate this, switch to using static class singleton's, or cache the metadata / column hit for the db escaping / query purposes.
@@ -167,7 +167,7 @@ public function is_tracker_encounter_exist($apptdate, $appttime, $pid, $eid) | |||
* @param string $enc_id | |||
* @return bool|int | |||
*/ | |||
public function manage_tracker_status($apptdate, $appttime, $eid, $pid, $user, $status = '', $room = '', $enc_id = '') | |||
public static function manage_tracker_status($apptdate, $appttime, $eid, $pid, $user, $status = '', $room = '', $enc_id = '') |
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.
There is a $this used in line 273, which will likely break. Not sure how to rework that to work for a static call. @adunsulag thoughts?
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.
We'd need to use a singleton instance (have a static $instance property that is initialized once), or skip refactoring this method if its not used in the flowboard.
Fixes #7329
Short description of what this resolves:
Changes proposed in this pull request: