BenchmarkTools punning `Base.time`?

[garborg]

It seems like BenchmarkTools is unique in punning Base.time to mean something other than seconds since epoch.

julia> m
BenchmarkTools.TrialEstimate: 
  time:             4.810 ms
  gctime:           0.000 ns (0.00%)
  memory:           1.66 MiB
  allocs:           68937

julia> time(m)
4.809736e6

help?> time
search: time Timer time_ns timedwait mtime ctime @time @timev @timed gctime @btime f_times f_subtimes istextmime trim selectdim permutedims permutedims! StridedMatrix

  time(t::TmStruct)

  Converts a TmStruct struct to a number of seconds since the epoch.

  ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

  time()

  Get the system time in seconds since the epoch, with fairly high (typically, microsecond) resolution.

julia> methods(time)
# 7 methods for generic function "time":
[1] time(t::BenchmarkTools.Trial) in BenchmarkTools at /home/sean/.julia/packages/BenchmarkTools/eCEpo/src/trials.jl:53
[2] time(t::BenchmarkTools.TrialEstimate) in BenchmarkTools at /home/sean/.julia/packages/BenchmarkTools/eCEpo/src/trials.jl:126
[3] time(t::BenchmarkTools.TrialRatio) in BenchmarkTools at /home/sean/.julia/packages/BenchmarkTools/eCEpo/src/trials.jl:154
[4] time(t::BenchmarkTools.TrialJudgement) in BenchmarkTools at /home/sean/.julia/packages/BenchmarkTools/eCEpo/src/trials.jl:201
[5] time(group::BenchmarkGroup) in BenchmarkTools at /home/sean/.julia/packages/BenchmarkTools/eCEpo/src/groups.jl:73
[6] time() in Base.Libc at libc.jl:247
[7] time(tm::Base.Libc.TmStruct) in Base.Libc at libc.jl:240

[vchuravy]

It is used as an accessor to the time spend on a benchmark? I am not sure what to do with this issue? Do you mean we should not import Base.time?

[garborg]

Not importing Base.time was my thought, but I understand it may not be worth doing anything about.

It was confusing to me to have those be the same function, especially when this package returns milliseconds rather than seconds.

That said, I'm probably on the extreme end of the spectrum where avoiding punning is helpful for productivity (via ability to remember / trust / reason about code).

(I'm also worried generally about the prospects of generating reasonably sized apps, and being strict about punning keeps the ecosystem from making tree shaking any less effective than it already will be, but that's admittedly not the most relevant to this individual case.)

Feel free to close if you think 'wont fix' is the best path.

[ararslan]

In my experience, using Base.time as an accessor is not all that uncommon; DSP.jl does it as well, for example.

[garborg]

Do you think adding a function time end (in Base) with a docstring that clarifies the catch-all behavior makes sense?

[ararslan]

Eh, I dunno. Tbh I don't find anything wrong/objectionable with the current situation.

[garborg]

On reflection, time seems like it fits in the 'probably too generic to be exported from Base, but too late to remove until 2.0' category, so suggesting adding the doc seems undesirable. I'll close this, possibly add an issue about removing it from base for 2.0, if no similar issue else exists.

[ararslan]

I'd pretty strongly oppose unexporting it from Base, I don't know why that would be a desirable solution to this.

[garborg]

I've still not come around, but this has veered into 'draining maintainer resources without providing me any benefit either' territory -- if it doesn't click for me eventually, I'll go to the forums for opt-in discussion and education to figure it out.