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

Suggestion, Implement "time" #117

Closed
etu opened this issue Jun 12, 2012 · 35 comments
Closed

Suggestion, Implement "time" #117

etu opened this issue Jun 12, 2012 · 35 comments

Comments

@etu
Copy link
Contributor

etu commented Jun 12, 2012

Like in bash:

$ time <do something that takes time>
...

real 0m0.000s
user 0m0.000s
sys 0m0.000s
$
@maxfl
Copy link
Contributor

maxfl commented Jun 12, 2012

time should not be implemented, because it's a system util
in arch/debian/ubuntu it is in the package 'time'

@etu
Copy link
Contributor Author

etu commented Jun 12, 2012

I can say it's built-in in bash. But, found package in gentoo too, so I guess that's fine.

@etu etu closed this as completed Jun 12, 2012
@justinmayer
Copy link

I think @etu might be onto something. As far as I can tell, the time command doesn't work with fish functions, at least on Mac OS X 10.8.2:

~> time myfishfunction
myfishfunction: No such file or directory

The time command produces the expected behavior when running an equivalent bash function. Unless I'm missing something, I think perhaps this should indeed be investigated further.

@JanKanis
Copy link
Contributor

It would be nice for this to work, but that requires making a builtin time. Labeling this as an enhancement for fish-future

@JanKanis JanKanis reopened this Jan 20, 2013
@terlar
Copy link
Contributor

terlar commented Mar 25, 2013

👍

@uggedal
Copy link
Contributor

uggedal commented May 10, 2013

If you need to time fish functions:

time -p fish -c 'chemical_element_to_symbol oxygen'
o
real 0.03
user 0.00
sys 0.02

@etu
Copy link
Contributor Author

etu commented May 10, 2013

Yes, that probably works. But does not produce a fair result. Because launching another instance of the shell means overhead.

So in theory, timing of all fish-functions would be equally slow (if you can trust that fish always takes the same time to launch). Which might not be true since we all do have different configs, different hardware, and stuff. I for instance set my own $PATH in fish-config, which not only sets a variable, it also checks for duplicates in the variable, and so on.

We would have a better result with better performance if we had a builtin for it.

And @uggedal, Your hack is a fine hack, so I rewrote my function "time" to use it. I made a function because I always want -p on the command to be there.

function time --description 'Wrapper for time'
    /usr/bin/time -p /bin/fish -c $argv
end

@ghost
Copy link

ghost commented May 11, 2013

Because time is... well, time-sensitive, having a builtin for it might actually make sense. That conceptually works but has some overhead in loading another fish process.

@tcrayford
Copy link

+1 on this. Not being able to accurately time builtin functions is frustrating

@mrak
Copy link
Contributor

mrak commented Oct 29, 2013

👍 I'd love it for making my complicated prompt functions faster

@nsmgr8
Copy link

nsmgr8 commented Dec 16, 2013

The custom time function that wrap the /usr/bin/time is not a good solution for me. It creates a new shell that does not have the current environment. Hence certain types of command will fail. For example, a python script that is running from a virtualenv.

@jcelliott
Copy link
Contributor

I've been using fish for a month and haven't needed it, but when I needed it today it was frustrating not having it. Another +1 for a built-in time.

@ridiculousfish
Copy link
Member

We can probably leverage the existing profiling support to make for a really awesome version of time.

@matejnt
Copy link

matejnt commented Mar 28, 2014

I think using the actual time program coincides with fish's design principles and should probably not be changed.

@dideler
Copy link
Contributor

dideler commented Jun 7, 2014

@nanthiel can you explain what you mean? Which design principle?

I'm also in favour of a built-in time command, in the mean time I'll use the wrapper function.

dideler added a commit to dideler/dotfiles that referenced this issue Jun 7, 2014
@KamilaBorowska
Copy link
Contributor

@nanthiel: This particular design principle you refer to (the law of minimalism) doesn't exist since 2.0.0 (since I complained about adding as echo a builtin). It disallowed things like echo builtin which were needed for performance reasons. Instead, it was replaced with the law of responsiveness which even recommends implementing often used built-ins for performance. I still disagree with doing that (why echo even supports -s?), but it had to be done.

But I think "the law of minimalism" should be considered anyway. The reason for it being removed is allowing the built-ins to be implemented for performance reasons. Except I'm not sure how it applies to time.

@matejnt
Copy link

matejnt commented Jun 16, 2014

@xfix: Sorry, I wasn't aware of the principle change. I do agree time is fine as a wrapper, however. It isn't performance critical anyway.

@danielb2
Copy link
Contributor

👍

$ which time
/usr/bin/time
$ alias foo ls
$ time foo
foo: No such file or directory
        0.00 real         0.00 user         0.00 sys

This works fine in bash as others have stated.

@pwr22
Copy link

pwr22 commented Nov 3, 2014

I agree that the limitations given by @nsmgr8, @etu and others above support implementing a built-in

@ryanhiebert
Copy link

AFAICT, the wrapper doesn't work for fish functions that aren't defined in the shell config. That means that I can't time functions that are only in the current shell.

So another 👍 from me to have it be builtin that is capable of running functions.

@krader1961
Copy link
Contributor

FWIW, I agree with @ridiculousfish that

We can probably leverage the existing profiling support to make for a really awesome version of time.

The fish -p switch is useful for evaluating the performance of an entire fish script (or interactive session) but isn't useful for evaluating the performance of a particular function in real-time. I'm not convinced that the time command should become a builtin since the time elapsed and CPU time doesn't provide useful insight into why a function is slow. It would, however, be very useful if there was a way to dynamically invoke performance profiling, ala -p, when running a specific command (i.e., function). Without including all the overhead of starting the shell, constructing a prompt, etcetera.

@sanmai-NL
Copy link

This is already implemented:
Under Fish version 2.2.0:

sleep 10
echo $CMD_DURATION

10003

The issue can be closed ...

@krader1961
Copy link
Contributor

It seems like this should work but doesn't:

function time
    eval (string escape $argv)
        echo command took $CMD_DURATION ms
end

For some reason $CMD_DURATION doesn't reflect how long the eval line took to execute. It appears to be the time required by the preceding command. That is, it appears to be updated only at the top-most execution context. So you can't actually use it within a function to find out how long the most recent command run by the function took to complete. I haven't yet looked at the source so it's hard to know if this is intentional or not. Certainly the documentation makes no mention of this limitation.

@etu
Copy link
Contributor Author

etu commented Apr 20, 2016

Nowadays I mostly use:

function ltime --description 'Print command duration in seconds for last command'
    echo (echo 'scale=3; ' $CMD_DURATION ' / 1000' | bc)"s"
end

and run it after the command is completed. This assumes 2.2.0 or newer to work well. So my need for this is kinda dead.

ltime of course stands for "last time" or something like that.

@ghost
Copy link

ghost commented Apr 20, 2016

AFIK $CMD_DURATION is the epoch since the last command, not function. So, while @etu's is not forking a new fish there, I wonder if there is any real improvement over just time fish -c "..."?

: pensive :

@faho
Copy link
Member

faho commented Apr 21, 2016

@bucaran: Yeah, that it doesn't include fish's startup time. Remember that fish always sources all of its configuration files.

Like @krader1961 noticed "$CMD_DURATION" is updated too late (which means you always need to check it after the next prompt, as a separate command), but that might not be unfixable.

But a much more awesome solution here would be what has already been proposed - a way to get our already existing profile stuff and apply it only to a certain command, e.g.

profile for i in (seq 1000); fish -ic 'exit'; end

And then time as a simpler way to get just the summary.

@jichu4n
Copy link

jichu4n commented Jun 19, 2016

Shameless plug: I made a fish-command-timer plugin to do this - it makes fish print out the execution time of each command line. Please feel free to check it out!

Code: jichu4n/fish-command-timer

Screenshot:
fish-command-timer screenshot

@faho faho mentioned this issue Jun 25, 2016
@whitelynx
Copy link

So, starting from some of @jichu4n's earlier code (before he switched fish-command-timer over to using $CMD_DURATION), I came up with this function to hack around the problem:

function realtime
	set -l command_start_time (date '+%s%N')
	eval (string escape $argv)
	set -l command_end_time (date '+%s%N')
	set -l command_duration (math "$command_end_time - $command_start_time")

	set -l minutes (math "$command_duration / 60000000000")
	set -l seconds (math "$command_duration % 60000000000 / 1000000000")
	set -l milliseconds (math "$command_duration % 1000000000 / 1000000")

	printf '\nreal\t%dm%d.%03ds\n' $minutes $seconds $milliseconds
end

It only tells you the "real" time (wall clock time) of the given command, not the user or system CPU time like bash's time command does, although that's the same as $CMD_DURATION anyway.

The big advantage over $CMD_DURATION is that you can use it in a loop, which is rather important for benchmarking. As an added bonus, it outputs in the same format as bash's time command. (minus the user and sys lines)


Regardless, I think the idea of a built-in command utilizing the existing profiling support is a very good idea; it's the only way we'd be able to get accurate insight into something like user/system CPU time, and it could easily give us more information too.

@pgan002
Copy link

pgan002 commented May 28, 2017

Is it possible to make CMD_DURATION hold the runtime of the last function if the last command was a function, otherwise the last command? This allows the user to know the runtime after the command is executed, unlike a builtin which requires the user to think of it before executing the command.

We could even make DURATION an array containing the runtime of (1) the last function, (2) block, (3) individual command. If there is no block or function, DURATION would contain three identical elements.

Using three variables CMD_DURATION, FUNC_DURATION, BLOCL_DURATION feels like polluting the variable space.

Another option is to replace CMD_DURATION by parameters to the status builtin.

@JoshCheek
Copy link

My system didn't have subsecond options for the date command, so I edited @whitelynx's script. Also added exit status and printed the result to stderr:

function realtime
       set -l start    (date '+%s')
       eval (string escape $argv)
       set -l _status  $status
       set -l stop     (date '+%s')
       set -l duration (math "$stop - $start")
       set -l minutes  (math "$duration / 60")
       set -l seconds  (math "$duration % 60")
       printf 'real\t%dm%ds\n' $minutes $seconds 1>&2
       return $_status
end

@devurandom
Copy link
Contributor

That script does not appear to work with command line arguments:

> realtime planck -e '(println "Hello World")'
string escape: Unknown option “-e”
~/.config/fish/functions/realtime.fish (line 2):
in command substitution
        called on line 0 of file ~/.config/fish/functions/realtime.fish

in function “realtime”
        called on standard input
        with parameter list “planck -e (println "Hello World")”
[...]

I am using Fish Shell version 2.6.0.

@zanchey
Copy link
Member

zanchey commented Nov 22, 2017

Change string escape $argv to string escape -- $argv.

@faho
Copy link
Member

faho commented Dec 19, 2019

This has now been implemented in e5e66ac.

@zanchey zanchey modified the milestones: fish-future, fish 3.1.0 Dec 19, 2019
@anrddh
Copy link

anrddh commented Feb 21, 2020

Is this a bug?

~ $ builtin time
fish: Unknown builtin “time”
builtin time
        ^
~ $ builtin time

I'm running fish 3.10 btw.

@zanchey
Copy link
Member

zanchey commented Feb 21, 2020

@anrddh, yes, see #6598.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests