-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Replace System.currentTimeMillis() by System.nanoTime() #509
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
Replace System.currentTimeMillis() by System.nanoTime() #509
Conversation
- System.nanoTime() is the best way to measure elapsed time in Java. - It gives a resolution on the order of microseconds The System.currentTimeMillis() is used when calculating absolut time.
- Cover when the profile is not started/stopped
|
cloudstack-pull-requests #629 SUCCESS |
| public long start() { | ||
| startTickInMs = System.currentTimeMillis(); | ||
| startTickInMs = System.nanoTime(); |
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.
I'd use a new name for the var, like startTickInNanos (and of course stopTickInNanos)
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 nanoTime() is given in Microseconds. So, the MS still fits in.
Cheers,
Wilder
|
Before we continue in a face-to-face chat, which would be better, just few point to take into account:
The comments above are only related to the fact that the Profiler class is being exploited inside ACS. Concerning the getDuration(). I agree that once it is taken into account to calculate milliseconds, it will be flaw. In that case, it's too bad I trusted the existing test - which has a glitch in the assertion. My suggestion is to refactor the getDuration(), renaming it into getDurationInMillis() and create a getDuration() which returns the nanotime - microseconds. The test has to be fixed as well. |
- Cover when the profile is not started/stopped Signed-off-by: wilderrodrigues <[email protected]> This closes apache#509
Addresses: shapeblue/cloudstack-apple#490 Adds framework layer change to allow retrieving and storing IOPS stats for storage pools. Custom `PrimaryStoreDriver` can implement method - `getStorageIopsStats` for returning IOPS stats. Existing method `getUsedIops` can also be overridden by such plugins when only used IOPS is returned. For testing purpose, implementation has been added for simulator hypervisor plugin to return capacity and used IOPS for a pool. For local storage pool, implementation has been added using iostat to return currently used IOPS. StoragePoolResponse class has been updated to return IOPS values which allows showing IOPS values in UI for different storage pool related views and APIs. Schema changes: [fr83-schema-changes.sql.txt](https://github.com/user-attachments/files/18006838/fr83-schema-changes.sql.txt) Upstream PR: apache#10034 --------- Signed-off-by: Abhishek Kumar <[email protected]>
Add unit tests to cover negative cases
Test Environment
Tests successfully executed: