-
Notifications
You must be signed in to change notification settings - Fork 39
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
parse buffer from different bases and data unit #754
Conversation
now := time.Now() | ||
diff := float64(now.UnixNano()-previousDebugTime) / 1000000 |
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.
@urbanishimwe I think we should leave diff here. Why did you remove it?
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.
Printing diff in Nanoseconds
! I don't think it's very helpful in the user's perspective. If you can see only the current time of a debug(in the same way of log stdlib), it's very enough to take relevant action.
Why do you think? Cc: @buger
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 thing is that the Debug function was doing 2 different things. First is debug output, and the second is performance measurement. So you can wrap your code into 2 Debug calls, and can see how much time it took to run.
debugMutex.Unlock() | ||
|
||
fmt.Printf("[DEBUG][PID %d][%d][%fms] ", os.Getpid(), now.UnixNano(), diff) | ||
diff := now.Sub(previousDebugTime).String() |
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.
If you leave it as "ns" then in the debug call on line 253 at least say that it is ns
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 built in Duration.String()
format well by giving it relevant prefix
- binary, hexadecimal, octal, decimal - kb, mb, gb, tb
All flags that expect buffer as input i.e. `--output-file-size-limit`, `--output-file-max-size-limit`, `--copy-buffer-size` and `input-raw-buffer-size` can now parse inputs from differents bases and data units like: `10mb`, `10kb`, `100gb`, `18tb`, `11839023`.... data units and bases are case insensitive, the parser accepts only the format of [Go integer literals](https://golang.org/ref/spec#Integer_literals)
All flags that expect buffer as input i.e.
--output-file-size-limit
,--output-file-max-size-limit
,--copy-buffer-size
andinput-raw-buffer-size
can now parse inputs from differents bases and data units like:10mb
,10kb
,100gb
,18tb
,11839023
....data units and bases are case insensitive, the parser accepts only the format of Go integer literals