dnrajugade / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

TimeSpan (TimeUnit + long) class #631

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Quite frequently, I have to add a duration as parameter to a method, and feel 
forced to add two parameters: java.util.concurrent.TimeUnit  as well as an 
int/long to hold the duration. And every time it makes me cringe, and consider 
whether to just add the int, and name it durationMillis (or such).
Firstly, a duration should not be broken into two discrete components, secondly 
the API shouldn't force a time unit upon the API user.

What Im looking for is an immutable TimeUnit and and long all rolled into one 
Duration class, that would have suitable:
* toString()
* equals()
* Comparable.compareTo()
- as well as conversions to any alternate TimeUnit

Wouldn't such a Duration type be well suited for Guava?

Original issue reported on code.google.com by morten.h...@gmail.com on 22 May 2011 at 9:34

GoogleCodeExporter commented 9 years ago
Isn't there something like that in JodaTime or in the JSR for Time aimed for 
JDK 8?

Original comment by gscerbak@gmail.com on 22 May 2011 at 9:41

GoogleCodeExporter commented 9 years ago
+1 for using JodaTime's Duration class: 
http://joda-time.sourceforge.net/apidocs/org/joda/time/Duration.html

Original comment by kurt.kluever on 22 May 2011 at 10:22

GoogleCodeExporter commented 9 years ago
-1 for Joda Time Duration class.

The Joda Time is aligned with ISO-8601 as part of a complete set of APIs to
support time and calendar.
TimeUnit, on the other hand, being part of java.util.concurrent, is aimed at
thread/processor/timer related APIs.

What I'm looking for is a way of avoiding a split-up of duration and unit
when calling java.util.concurrent API, and Joda Time provides none of that:

   1. Joda Time Duration does NOT contain/convert to TimeUnit based units.
   2. Joda Time Duration does not support sub-milliseconds granularity

A TimeUnit/long wrapper would be lightweight and general purpose, which fits
nicely into Guava, I think.

/Morten

Original comment by morten.h...@gmail.com on 23 May 2011 at 7:13

GoogleCodeExporter commented 9 years ago
I would agree with Morten, especially since I saw the classes like Stopwatch 
making their first steps in the trunk of Guava. This Stopwatch would be the 
first class to benefit directly from this wrapper.

Original comment by ogregoire on 23 May 2011 at 10:51

GoogleCodeExporter commented 9 years ago
I agree with this need. I call this ShortDuration. My current incarnation can 
only handle durations of up to 100 days, but has picosecond precision. I need 
it for caliper.

Original comment by kevinb@google.com on 24 May 2011 at 4:23

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 13 Jul 2011 at 6:18

GoogleCodeExporter commented 9 years ago
This is reasonably likely for release 11 (since I selfishly need it in 
caliper.) :)

Original comment by kevinb@google.com on 13 Jul 2011 at 6:57

GoogleCodeExporter commented 9 years ago
Is this still likely for 11? It would definitely be nice to have.

Original comment by cgdec...@gmail.com on 22 Nov 2011 at 3:33

GoogleCodeExporter commented 9 years ago
Sadly, no, it's gotten terribly delayed.

Original comment by kevin...@gmail.com on 29 Nov 2011 at 3:47

GoogleCodeExporter commented 9 years ago
I would be happy to take this on...is there any particular complicating factor?

Original comment by wasserman.louis on 6 Dec 2011 at 4:16

GoogleCodeExporter commented 9 years ago
Hmm. Surely we want this? It would have been nice if this was in 
java.util.concurrent from day 1, but now there is a sea of signatures accepting 
this pair.

And it's not too clear to me that there is a usability win here, (5L, SECONDS) 
compared to (ShortDuration.of(5L, SECONDS)). The former is "convention" by now 
anyway.

Nor does this help with the implementation of time-related classes; we 
canonicalize any user-supplied duration to the same unit, and a long is enough 
for that. No idea why caliper needs pico precision - perhaps expecting to 
invoking a trial and finishing... in under 3 machine cycles? :-)

Original comment by jim.andreou on 7 Dec 2011 at 1:59

GoogleCodeExporter commented 9 years ago
The one use case that probably is the most convincing would be the situation 
where you would like to add a duration as a constant.

Examples:

Current Example 1 (split unit and value):
  private static final long TIMEOUT_VALUE = 2L; // you need to consult the TIMEOUT_UNIT before determining value
  private static final TimeUnit TIMEOUT_UNIT = TimeUnit.MINUTES;
  ...
  logger.debug("Waiting for " + TIMEOUT_VALUE + " " + TIMEOUT_UNIT); // tedious
  future.get(TIMEOUT_VALUE, TIMEOUT_UNIT);

Current Example 2 (split unit and value):
  private static final TIMEOUT_MILLISECONDS = 120000L; // difficult to decipher as 2 minutes, what if someone needs sub-millisecond value
  ...
  logger.debug("Waiting for " + TIMEOUT_MILLISECONDS + " milliseconds");
  future.get(TIMEOUT_MILLISECONDS, TimeUnit.MILLISECONDS);

Current Example 3 (split unit and value):
  private static final TIMEOUT_NANOSECONDS = TimeUnit.NANOSECONDS.convert(2, TimeUnit.MINUTES); // Semantically clearer
  ...
  logger.debug("Waiting for " + TIMEOUT_NANOSECONDS + " nanoseconds"); // difficult to decipher value in log
  future.get(TIMEOUT_NANOSECONDS, TimeUnit.NANOSECONDS);

ShortDuration Example 3 (using ShortDuration):
  private static final ShortDuration TIMEOUT = ShortDuration.minutes(2); // would look good with static import, too
  ...
  logger.debug("Waiting for " + TIMEOUT);
  future.get(TIMEOUT);

Original comment by morten.h...@gmail.com on 7 Dec 2011 at 8:30

GoogleCodeExporter commented 9 years ago
Louis -- awesome!

Here's what it needs:

1. Instead of having a hard limit of 100 days and picosecond granularity, can't 
it just return a different, BigDecimal-based class when it's outside that 
range? For some reason I didn't think of that.  If we do this it needs another 
name though, and I'm a little queasy about using Duration which Joda-Time 
already has.

2. I never wrote tests unless you count the supercrappy ones that you can find 
in the caliper source tree right now.

I'll email you the code I left off with.

Original comment by kevinb@google.com on 9 Dec 2011 at 11:32

GoogleCodeExporter commented 9 years ago
"Jim" :-) -- Caliper just wants to be able to compute and report a value like 
"13.17 ns" -- so there's sub-nanosecond precision there.

Many of the good uses for this object have to do with its ability to display as 
and parse from a String, e.g. check out CacheBuilderSpec.

I do *not* propose that we should automatically start replacing (long, 
TimeUnit) signatures with (ShortDuration) ones as general practice. Sometimes 
that might be useful, sometimes not.

Original comment by kevinb@google.com on 9 Dec 2011 at 11:40

GoogleCodeExporter commented 9 years ago

Original comment by fry@google.com on 10 Dec 2011 at 4:11

GoogleCodeExporter commented 9 years ago
Kevin: "I do *not* propose that we should automatically start replacing (long, 
TimeUnit) signatures with (ShortDuration) ones as general practice."

May I ask, why not?

I cannot think of a single case where a split duration (long, TimeUnit)...

doSomething(2, TimeUnit.SECONDS);

... would would be preferable to a single ShortDuration ...

doSomething(ShortDuration.seconds(2));

Except for the fact that it would be different to what the java.util.concurrent 
library methods currently use, but that shouldn't, in my opinion, prevent Guava 
from doing it better.

Except, of course, where java.util.concurrent interfaces mandates the split 
duration parameters.

Original comment by morten.h...@gmail.com on 16 Dec 2011 at 9:55

GoogleCodeExporter commented 9 years ago
I just mean that I don't personally advocate going and doing all that. That's 
not my intent in introducing this new class -- that it should become "the new 
way to do (long,TimeUnit) everywhere."

That doesn't mean I'm arguing *against* it though. 

Original comment by kevinb@google.com on 28 Jan 2012 at 7:24

GoogleCodeExporter commented 9 years ago
This is the strangest thing: it seems sensible to drop the "short" requirement, 
but if I do, I can't for the life of me think what to name the class!  It feels 
very wrong to poach a name from Joda.  I mean, we could call it ElapsedTime?

Original comment by kevinb@google.com on 2 Mar 2012 at 8:38

GoogleCodeExporter commented 9 years ago
WaitDuration?  It's typically used for "how long do you wait"...

Original comment by wasserman.louis on 2 Mar 2012 at 8:45

GoogleCodeExporter commented 9 years ago
TimeSpan?

Original comment by stephan...@gmail.com on 2 Mar 2012 at 8:54

GoogleCodeExporter commented 9 years ago
I think both ElapsedTime and WaitDuration are too specific to possible uses of 
the class rather than describing just what it is. "ElapsedTime" implies that 
it's an amount of time that has already elapsed and was measured, while 
"WaitDuration" implies that the duration is an amount of time to wait. While 
each of those is applicable to some use cases for such a class, the class 
encompasses both use cases and more and should probably have a name that 
reflects that.

Original comment by cgdec...@gmail.com on 2 Mar 2012 at 8:55

GoogleCodeExporter commented 9 years ago
I'm curious about a couple things.  Sorry if these are dumb questions.

1. Will this type have methods like get(TimeUnit) or get[Nanos/Micros/etc]() 
and if so, what would their return types be?  long, Number, BigInteger, or 
BigDecimal?

2. I think I understand why/how producers of this type would use it, but how 
would consumers typically use it?  Say you've been passed one of these and it 
represents a timeout.  Would you periodically construct a new one based on the 
current time and compare it to the timeout?  e.g.

  if (ShortDuration.seconds(elapsedSeconds).isGreaterThan(timeout)) {
    // fail
  }

Original comment by michael.hixson@gmail.com on 6 Mar 2012 at 10:40

GoogleCodeExporter commented 9 years ago
How about TimeInterval or TimeSpan as given in Comment 20 as a name?  I agree 
with Comment 21 that WaitDuration and ElapsedTime both hint at how the class is 
used while TimeInterval or TimeSpan describe the class without implying its use.

Also, may I suggest a fromString method?  I know strings are overused, but a 
fromString method could make reading in a time interval from say a config file 
very convenient.  The format of accepted strings could be something like a long 
followed by any whitespace followed by a TimeUnit (supporting either singular 
or plural units).

Original comment by jmsig...@gmail.com on 4 May 2012 at 9:19

GoogleCodeExporter commented 9 years ago
Yup, TimeSpan is the current choice, and one of the best things about the code 
we have so far is its fromString() functionality (which we can drop into, for 
example, CacheBuilderSpec).

Original comment by kevinb@google.com on 6 May 2012 at 3:29

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 17 May 2012 at 11:16

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 30 May 2012 at 7:43

GoogleCodeExporter commented 9 years ago
I'd be interested to hear folks' requirements for this class that aren't 
satisfied by the current rev of JSR-310's Duration.
https://github.com/ThreeTen/threeten/blob/master/src/main/java/javax/time/Durati
on.java

Original comment by brianfromoregon on 7 Aug 2012 at 8:20

GoogleCodeExporter commented 9 years ago
Oh well there's one in #14, representing sub-nano precision. 

Original comment by brianfromoregon on 7 Aug 2012 at 8:28

GoogleCodeExporter commented 9 years ago
Is anyone actively working on this issue.
Otherwise I would consider making an implementation myself, and submitting it 
to Guava as a proposal.

I guess the design would include:

- Immutable implementation
- Static factory methods for creating instance of common TimeUnits, e.g. 
TimeSpan.minute(2).
- Static factory methods for creating instance from TimeUnit/long combination, 
e.g. of(Timeunit unit, long duration)
- Static factory methods for creating instance from a readable String 
representation, as in CacheBuilderSpec
- Conversions to long: asLong(TimeUnit unit)
- implements Comparable<TimeSpan>
- Possible convenience instance methods isGreaterThan(TimeSpan timeSpan) (... 
etc), providing more readable client code than using Comparable.compareTo()
- Instance method toString(TimeUnit unit) providing string representation using 
unit (integer?)

Questions:
- Should toString() normalize the reported TimeUnit, or should the TimeSpan 
retain information about which time unit was used to create it, and use that 
unit when using toString.

Original comment by morten.h...@gmail.com on 8 Aug 2012 at 11:26

GoogleCodeExporter commented 9 years ago
I suggest putting the addition in the implementation. Maybe in a TimeSpans 
class?

But it might be interesting as well to make a list of all Guava classes that 
could benefit from it.

Original comment by ogregoire on 8 Aug 2012 at 11:42

GoogleCodeExporter commented 9 years ago
> Is anyone actively working on this issue.

Kevin, is this still on your plate?

Here's the existing Caliper ShortDuration class:

http://code.google.com/p/caliper/source/browse/caliper/src/main/java/com/google/
caliper/util/ShortDuration.java

Original comment by cpov...@google.com on 8 Aug 2012 at 5:48

GoogleCodeExporter commented 9 years ago
Well, it would be a cause for celebration if it gets onto someone else's plate. 
 I could give a 5-minute rundown of what aspects of ShortDuration I do and 
don't think are actually worthy to carry over.

Original comment by kevinb@google.com on 8 Aug 2012 at 6:22

GoogleCodeExporter commented 9 years ago
As a data point, my employer makes good use of a simplified interface (100+ 
call sites):

@Immutable
public final class TimeValue {
    private final long time;
    private final TimeUnit timeUnit;

    private TimeValue(final long time, final TimeUnit timeUnit);
    public long getTime();
    public TimeUnit getTimeUnit();
    public String toString();

    public static TimeValue nanoseconds(final long value);
    public static TimeValue microseconds(final long value);
    public static TimeValue milliseconds(final long value);
    public static TimeValue seconds(final long value);
    public static TimeValue minutes(final long value);
    public static TimeValue hours(final long value);
    public static TimeValue days(final long value);

    public long toNanos();
    public long toMicros();
    public long toMillis();
    public long toSeconds();
    public long toMinutes();
    public long toHours();
    public long toDays();
}

I tried implementing arithmetic and comparison but these did not simplify call 
sites much.  Note that there is not enough precision to represent all values, 
e.g., TimeValue.days(1000).toNanos() > Long.MAX_VALUE, although this has not 
been an issue in practice.

Original comment by arg...@gmail.com on 26 Sep 2012 at 12:21

GoogleCodeExporter commented 9 years ago
Any news on this? Or should we stick to the new Java 8 java.time.Duration class?

Original comment by ogregoire on 31 Mar 2014 at 1:28

GoogleCodeExporter commented 9 years ago
We have a Duration class: 
https://github.com/airlift/airlift/blob/master/units/src/main/java/io/airlift/un
its/Duration.java

It is very useful for human-readable config files: Duration.valueOf("5m")

And a related class, DataSize: 
https://github.com/airlift/airlift/blob/master/units/src/main/java/io/airlift/un
its/DataSize.java

Original comment by electrum on 25 Apr 2014 at 10:42

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:15

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:18

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:09