Skip to content
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

Measuring unit should be ms, not in seconds #155

Open
nengxu opened this issue Feb 7, 2014 · 12 comments
Open

Measuring unit should be ms, not in seconds #155

nengxu opened this issue Feb 7, 2014 · 12 comments

Comments

@nengxu
Copy link

nengxu commented Feb 7, 2014

The measuring unit for time seems to be in seconds. This may produce not useful statistics, for example:

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Database time - by sum                                            ┃ Hits ┃    Sum ┃   Mean ┃ StdDev ┃    Min ┃    Max ┃    95 %tile
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
IdentitiesController#myacct.JS                                    ┃    2 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
IdentitiesController#index.HTML                                   ┃    2 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
GatewaysController#check_node_connection.*/*                      ┃    6 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
SessionsController#new.HTML                                       ┃   15 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
HomeController#index.HTML                                         ┃    8 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
DoorsController#index.HTML                                        ┃    2 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
SystemsumController#index.HTML                                    ┃    3 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
IdentitiesController#default_display.XML                          ┃    5 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
GatewaysController#edit.HTML                                      ┃    7 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
SupportController#show.HTML                                       ┃    1 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
IdentitiesController#new.HTML                                     ┃    1 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
IdentitiesController#identity_profile_list.HTML                   ┃    1 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
IdentitiesController#myacct.HTML                                  ┃    2 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
RolesController#index.HTML                                        ┃    4 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
AccessGroupsController#index.HTML                                 ┃    6 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
AccessGroupsController#new.HTML                                   ┃    6 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s
AlarmsController#index.HTML                                       ┃    1 ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃  0.00s ┃ 0.00s-0.00s

Please change the unit to ms. Thanks.

@wvanbergen
Copy link
Owner

@barttenbrinke what do you think?

@jefflunt
Copy link
Contributor

Would either of these two solutions be acceptable to all parties?

Display ms label after floating point number when time is 0...1

Patch this range into two ranges so that it reads:

  when 0...1    "%0.2fms" % (time*1000)
  when 1...60    then "%0.02fs" % time

...which produces:

 > time = 0.0013
 > "%0.2fms" % (time*1000)
=> "1.30ms"

The effect is that the 'ms' label is only applied in the cases where @nengxu and others are going to care.

Display ms label after integer number when time is 0...1

Alternatively, show it as an integer number of milliseconds rather than a floating point at all:

  when 0...1    "%0ims" % (time*1000)

...which produces

 > time = 0.0013
 > "%0ims" % (time*1000)
=> "1ms"

@nengxu
Copy link
Author

nengxu commented Mar 28, 2014

would prefer #1, thanks.
On 2014-03-27 9:05 PM, "Jeff Lunt" [email protected] wrote:

Would either of these two solutions be acceptable to all parties?

Display ms label after floating point number when time is 0...1

Patch this range into two rangeshttps://github.com/wvanbergen/request-log-analyzer/blob/master/lib/request_log_analyzer/tracker/duration.rb#L41so that it reads:

when 0...1 "%0.2fms" % (time*1000)
when 1...60 then "%0.02fs" % time

...which produces:

time = 0.0013
"%0.2fms" % (time*1000)=> "1.30ms"

The effect is that the 'ms' label is only applied in the cases where
@nengxu https://github.com/nengxu and others are going to care.

Display ms label after integer number when time is 0...1

Alternatively, show it as an integer number of milliseconds rather than a
floating point at all:

when 0...1 "%0ims" % (time*1000)

...which produces

time = 0.0013
"%0ims" % (time*1000)=> "1ms"

Reply to this email directly or view it on GitHubhttps://github.com//issues/155#issuecomment-38887128
.

@jefflunt
Copy link
Contributor

So @nengxu your output might look something like this, when you have seconds and milliseconds sitting next to each other in the table:

Database time - by sum                                            ┃ Hits ┃    Sum ┃   blah ┃  blah ┃   blah
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
IdentitiesController#myacct.JS                                    ┃    2 ┃  1.05s ┃ 
IdentitiesController#myacct.JS                                    ┃    2 ┃ 1.03ms ┃

@jefflunt
Copy link
Contributor

Actually, I might be in favor of a 3rd option, which is to add a 3rd decimal place to the s label.

Here's a side-by-side comparison of adding an ms label vs adding a third decimal place in the html view:
screen shot 2014-03-27 at 11 58 15 pm

I think the third decimal place it easier to read because the numbers still line up visually as you would expect. This is equally true in fixed-width format as you can see below, especially because it makes comparison of the magnitude of numbers in any of the non-sort columns easier to see.

The tradeoff to the extra decimal place, of course, is that it takes up space that is especially prized in the fixed-width format.

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
View rendering time - by sum                                                 ┃ Hits ┃     Sum ┃    Mean ┃  StdDev ┃    Min ┃     Max ┃       95 %tile
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
SurveyorController#show.HTML                                                 ┃   28 ┃  14m27s ┃ 30.978s ┃ 41.149s ┃ 0.966s ┃   1m59s ┃   0.962s-1m59s
SurveyorController#edit.HTML                                                 ┃  292 ┃  10m58s ┃  2.253s ┃  2.565s ┃ 0.052s ┃ 20.268s ┃  0.151s-8.892s
PublicSurveyorController#edit.HTML                                           ┃   78 ┃   3m54s ┃  3.000s ┃  2.664s ┃ 0.597s ┃ 11.861s ┃ 0.603s-10.789s
SurveysController#show.JS                                                    ┃   78 ┃   1m51s ┃  1.419s ┃  2.236s ┃ 0.004s ┃  6.693s ┃  0.031s-6.442s
PublicController#index.HTML                                                  ┃   69 ┃   1m01s ┃  0.891s ┃  2.499s ┃ 0.003s ┃ 15.991s ┃  0.003s-8.892s
InvolvementsController#show.HTML                                             ┃   91 ┃ 45.799s ┃  0.503s ┃  0.879s ┃ 0.017s ┃  6.250s ┃  0.018s-4.977s
UsersController#dashboard.HTML                                               ┃   56 ┃ 45.670s ┃  0.816s ┃  2.082s ┃ 0.005s ┃  9.149s ┃  0.006s-6.546s
QuestionsController#create.JS                                                ┃   71 ┃ 36.444s ┃  0.513s ┃  0.199s ┃ 0.132s ┃  0.772s ┃  0.137s-0.743s
SurveysController#preview.JS                                                 ┃   43 ┃ 34.467s ┃  0.802s ┃  2.005s ┃ 0.010s ┃  6.293s ┃  0.010s-6.339s
StudiesController#settings.HTML                                              ┃   83 ┃ 14.792s ┃  0.178s ┃  0.743s ┃ 0.018s ┃  4.884s ┃  0.021s-4.819s
SurveysController#activate.JS                                                ┃    2 ┃ 13.648s ┃  6.824s ┃  0.173s ┃ 6.702s ┃  6.946s ┃  6.653s-6.982s

Either way, if @barttenbrinke and @wvanbergen approve of any of these three changes I'll be happy to submit a pull request for it.

@nengxu
Copy link
Author

nengxu commented Mar 28, 2014

That is perfect. Thanks.
On 2014-03-27 9:23 PM, "Jeff Lunt" [email protected] wrote:

So @nengxu https://github.com/nengxu your output might look something
like this, when you have seconds and milliseconds sitting next to each
other in the table:

Database time - by sum ┃ Hits ┃ Sum ┃ blah ┃ blah ┃ blah
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
IdentitiesController#myacct.JS ┃ 2 ┃ 1.05s ┃
IdentitiesController#myacct.JS ┃ 2 ┃ 1.03ms ┃


Reply to this email directly or view it on GitHubhttps://github.com//issues/155#issuecomment-38887791
.

@barttenbrinke
Copy link
Collaborator

@wvanbergen I think I should have looked at this PR a long time ago :-X
I don't want to widen the console version, so the number 1 is ok. @jefflunt : Could you wrap it up in a PR?

Greetings, Bart

@jefflunt
Copy link
Contributor

Sorry, I didn't mean to come off as pushy if I did - I just happened to have my head in the gem last night. You could still just say no to the request itself. :)

I'll get started on a PR for option 1, for your review. Thanks for the feedback.

@barttenbrinke
Copy link
Collaborator

@jefflunt No problem :) I was just telling @wvanbergen that I've been lazy :)

jefflunt added a commit to jefflunt/request-log-analyzer that referenced this issue Mar 30, 2014
- Altered the labeling of durations when <1 second
- Altered related specs that cover output to email
- Cleaned up a few lines that had trailing whitespace
@jefflunt
Copy link
Contributor

Submitted PR

@barttenbrinke
Copy link
Collaborator

@jefflunt Could you change the output from "40.18ms-41.50ms" to "40ms-42ms" ?
I'd rather see the Controller name than that kind of ms precision.

@jefflunt
Copy link
Contributor

Sure thing.
On Mar 31, 2014 4:03 AM, "Bart ten Brinke" [email protected] wrote:

@jefflunt https://github.com/jefflunt Could you change the output from
"40.18ms-41.50ms" to "40ms-42ms" ?
I'd rather see the Controller name than that kind of ms precision.

Reply to this email directly or view it on GitHubhttps://github.com//issues/155#issuecomment-39067288
.

jefflunt added a commit to jefflunt/request-log-analyzer that referenced this issue Mar 31, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants