cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wilderrodrigues <...@git.apache.org>
Subject [GitHub] cloudstack pull request: Replace System.currentTimeMillis() by Sys...
Date Tue, 23 Jun 2015 09:37:36 GMT
Github user wilderrodrigues commented on the pull request:

    https://github.com/apache/cloudstack/pull/509#issuecomment-114421017
  
    Before we continue in a face-to-face chat, which would be better, just few point to take
into account:
    
    * The Profiler class is used as a profiler, to just measure how long a code took to be
executed; used along with sleep calls - like a stop watch; and, still, as a monitor - there
are lock implementations based on it. It makes the Profile class a profile, a stop-watch and
a monitor.
    
    * From a profiler perspective, do a Star, wait for 1 second, then a Stop and check how
long it took to compute doesn't seem right to me.
    
    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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message