Skip to content

google-cloud-core: Optimize Timestamp.parseTimestamp(String) #4831

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.

Sign up for GitHub

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

Closed
olavloite wants to merge 11 commits into from
Closed

google-cloud-core: Optimize Timestamp.parseTimestamp(String) #4831

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7bd1485
optimize parsing of dates and timestamps
olavloite Mar 30, 2019
fee061b
fixed formatting
olavloite Mar 30, 2019
16657ee
fixed parsing nanoseconds from less than 9 digits
olavloite Mar 31, 2019
85aee11
switch statement is faster than if-statement
olavloite Apr 3, 2019
acf3c11
fixed formatting
olavloite Apr 3, 2019
b1feca4
added additional tests
olavloite Apr 4, 2019
be151f8
T and Z should be case insensitive
olavloite Apr 5, 2019
2cd776d
removed date parsing (moved to separate PR)
olavloite Apr 6, 2019
03384a0
Integer.parseInt is just as efficient
olavloite Apr 7, 2019
82a8257
added try-catch for NumberFormatException
olavloite Apr 9, 2019
63001ff
removed unnecessary else and put calculation on one line
olavloite Apr 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 54 additions & 15 deletions google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,15 @@

import static com.google.common.base.Preconditions.checkArgument;

import com.google.api.client.util.Preconditions;
import com.google.protobuf.util.Timestamps;
import java.io.Serializable;
import java.util.Date;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
import org.threeten.bp.Instant;
import org.threeten.bp.LocalDateTime;
import org.threeten.bp.ZoneOffset;
import org.threeten.bp.format.DateTimeFormatter;
import org.threeten.bp.format.DateTimeFormatterBuilder;
import org.threeten.bp.temporal.TemporalAccessor;

/**
* Represents a timestamp with nanosecond precision. Timestamps cover the range [0001-01-01,
Expand All @@ -49,15 +47,6 @@ public final class Timestamp implements Comparable<Timestamp>, Serializable {

private static final DateTimeFormatter format = DateTimeFormatter.ISO_LOCAL_DATE_TIME;

private static final DateTimeFormatter timestampParser =
new DateTimeFormatterBuilder()
.appendOptional(DateTimeFormatter.ISO_LOCAL_DATE_TIME)
.optionalStart()
.appendOffsetId()
.optionalEnd()
.toFormatter()
.withZone(ZoneOffset.UTC);

private final long seconds;
private final int nanos;

Expand Down Expand Up @@ -176,14 +165,64 @@ public com.google.protobuf.Timestamp toProto() {
return com.google.protobuf.Timestamp.newBuilder().setSeconds(seconds).setNanos(nanos).build();
}

private static final int[] POWERS_OF_10 = {
1, 10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000
};

/**
* Creates a Timestamp instance from the given string. String is in the RFC 3339 format without
* the timezone offset (always ends in "Z").
*/
public static Timestamp parseTimestamp(String timestamp) {
TemporalAccessor temporalAccessor = timestampParser.parse(timestamp);
Instant instant = Instant.from(temporalAccessor);
return ofTimeSecondsAndNanos(instant.getEpochSecond(), instant.getNano());
Preconditions.checkNotNull(timestamp);
final String invalidTimestamp = "Invalid timestamp: " + timestamp;
Preconditions.checkArgument(
timestamp.length() >= 19 && timestamp.length() <= 30, invalidTimestamp);
Preconditions.checkArgument(timestamp.charAt(4) == '-', invalidTimestamp);
Preconditions.checkArgument(timestamp.charAt(7) == '-', invalidTimestamp);
Preconditions.checkArgument(
timestamp.charAt(10) == 'T' || timestamp.charAt(10) == 't', invalidTimestamp);
Preconditions.checkArgument(
(timestamp.length() == 19 && (timestamp.charAt(18) != 'Z' || timestamp.charAt(18) == 'z'))
|| (timestamp.length() == 20
&& (timestamp.charAt(19) == 'Z' || timestamp.charAt(19) == 'z'))
|| (timestamp.charAt(19) == '.'
&& timestamp.length() > 20
&& (timestamp.charAt(20) != 'Z' || timestamp.charAt(20) == 'z')),
invalidTimestamp);
try {
int year = Integer.parseInt(timestamp.substring(0, 4));
int month = Integer.parseInt(timestamp.substring(5, 7));
int day = Integer.parseInt(timestamp.substring(8, 10));
int hour = Integer.parseInt(timestamp.substring(11, 13));
int minute = Integer.parseInt(timestamp.substring(14, 16));
int second = Integer.parseInt(timestamp.substring(17, 19));
int fraction = 0;
if (timestamp.length() > 20) {
if (timestamp.charAt(19) == '.') {
int endIndex;
if (timestamp.charAt(timestamp.length() - 1) == 'Z'
|| timestamp.charAt(timestamp.length() - 1) == 'z') {
endIndex = timestamp.length() - 1;
} else {
endIndex = timestamp.length();
}
if (endIndex - 20 > 9) {
throw new IllegalArgumentException(invalidTimestamp);
}
fraction = Integer.parseInt(timestamp.substring(20, endIndex));
// Adjust the result to nanoseconds if the input length is less than 9 digits (9 -
// (endIndex
// - 20)).
Copy link
Contributor

@sduskis sduskis Apr 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please have the mathematical formula on a single line, if possible. (9 - (endIndex - 20)).

fraction *= POWERS_OF_10[9 - (endIndex - 20)];
} else {
Copy link
Contributor

@sduskis sduskis Apr 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this else need to be here?

}
}
LocalDateTime ldt = LocalDateTime.of(year, month, day, hour, minute, second, fraction);
return ofTimeSecondsAndNanos(ldt.toEpochSecond(ZoneOffset.UTC), ldt.getNano());
} catch (NumberFormatException e) {
throw new IllegalArgumentException(invalidTimestamp, e);
}
}

private StringBuilder toString(StringBuilder b) {
Expand Down
112 changes: 112 additions & 0 deletions google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import static com.google.common.testing.SerializableTester.reserializeAndAssert;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;

import com.google.common.testing.EqualsTester;
import java.util.Calendar;
Expand All @@ -30,6 +31,7 @@
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.threeten.bp.DateTimeException;

/** Unit tests for {@link com.google.cloud.Timestamp}. */
@RunWith(JUnit4.class)
Expand All @@ -42,6 +44,8 @@ public class TimestampTest {
private static final long TEST_TIME_MILLISECONDS_NEGATIVE = -1000L;
private static final Date TEST_DATE = new Date(TEST_TIME_MILLISECONDS);
private static final Date TEST_DATE_PRE_EPOCH = new Date(TEST_TIME_MILLISECONDS_NEGATIVE);
private static final long TIMESTAMP_SECONDS_MAX = 253402300799L;
private static final long TIMESTAMP_SECONDS_MIN = -62135596800L;

@Rule public ExpectedException expectedException = ExpectedException.none();

Expand Down Expand Up @@ -179,19 +183,127 @@ public void testToString() {
@Test
public void parseTimestamp() {
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00Z")).isEqualTo(Timestamp.MIN_VALUE);
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.1Z"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_0000_0000));
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.01Z"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_000_0000));
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.100000000Z"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_0000_0000));
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.010000000Z"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_000_0000));
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.000000001Z"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1));
assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.999999999Z"))
.isEqualTo(Timestamp.MAX_VALUE);
assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.99999999Z"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MAX, 9999_9999_0));
assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.099999999Z"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MAX, 9999_9999));
assertThat(Timestamp.parseTimestamp(TEST_TIME_ISO))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0));
assertThat(Timestamp.parseTimestamp("2015-10-12T15:14:54.0Z"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0));
assertThat(Timestamp.parseTimestamp(Timestamp.ofTimeMicroseconds(20L).toString()))
.isEqualTo(Timestamp.ofTimeMicroseconds(20L));
assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.000020000Z"))
.isEqualTo(Timestamp.ofTimeMicroseconds(20L));
assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.00002Z"))
.isEqualTo(Timestamp.ofTimeMicroseconds(20L));
assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.000020Z"))
.isEqualTo(Timestamp.ofTimeMicroseconds(20L));
assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.00012340Z"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(0, 123400));
assertThat(Timestamp.parseTimestamp("2004-02-29T00:00:00Z")).isNotNull(); // normal leap year
assertThat(Timestamp.parseTimestamp("2000-02-29T00:00:00Z")).isNotNull(); // special leap year

parseInvalidTimestamp("");
parseInvalidTimestamp("TEST");
parseInvalidTimestamp("aaaa-01-01T00:00:00Z");
parseInvalidTimestamp("0001-bb-01T00:00:00Z");
parseInvalidTimestamp("0001-01-ccT00:00:00Z");
parseInvalidTimestamp("0001-01-01Tdd:00:00Z");
parseInvalidTimestamp("0001-01-01T00:ee:00Z");
parseInvalidTimestamp("0001-01-01T00:ee:00Z");
parseInvalidTimestamp("0001-01-01T00:00:ffZ");
parseInvalidTimestamp("0001-01-01T00:00:00.123aZ");
parseInvalidTimestamp("0001-1-1 00:00:00Z"); // missing 0
parseInvalidTimestamp("0001-01-01T00:00:00.Z"); // missing digits after .s
parseInvalidTimestamp("0001-01-01T00:00:00.1234567890Z"); // too long
parseInvalidTimestamp("0001-01-01T00:00Z"); // missing seconds
parseInvalidTimestamp("10000-01-01T00:00Z"); // year too long
parseTimestampOutOfRange("0000-01-01T00:00:00.1Z");
parseValidFormatInvalidDate("0001-00-01T00:00:00.1Z"); // month is zero
parseValidFormatInvalidDate("0001-01-00T00:00:00.1Z"); // day is zero
parseValidFormatInvalidDate("0001-13-01T00:00:00.1Z"); // invalid month
parseValidFormatInvalidDate("0001-01-32T00:00:00.1Z"); // invalid day
parseValidFormatInvalidLeapYear("0001-02-29T00:00:00.1Z"); // not a leap year
parseValidFormatInvalidLeapYear("1900-02-29T00:00:00.1Z"); // not a leap year
}

private void parseInvalid(String input, Class<? extends Exception> exception, String msg) {
try {
Timestamp.parseTimestamp(input);
fail("Expected exception");
} catch (Exception e) {
assertThat(e.getClass()).isEqualTo(exception);
assertThat(e.getMessage()).contains(msg);
}
}

private void parseInvalidTimestamp(String input) {
parseInvalid(input, IllegalArgumentException.class, "Invalid timestamp");
}

private void parseTimestampOutOfRange(String input) {
parseInvalid(input, IllegalArgumentException.class, "timestamp out of range");
}

private void parseValidFormatInvalidDate(String input) {
parseInvalid(input, DateTimeException.class, "Invalid value");
}

private void parseValidFormatInvalidLeapYear(String input) {
parseInvalid(input, DateTimeException.class, "not a leap year");
}

@Test
public void parseTimestampWithoutTimeZoneOffset() {
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00")).isEqualTo(Timestamp.MIN_VALUE);
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.1"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_0000_0000));
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.01"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_000_0000));
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.000000001"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1));
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.100000000"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_0000_0000));
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.010000000"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_000_0000));
assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.999999999"))
.isEqualTo(Timestamp.MAX_VALUE);
assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.99999999"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MAX, 9999_9999_0));
assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.099999999"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MAX, 9999_9999));
assertThat(Timestamp.parseTimestamp("2015-10-12T15:14:54"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0));
assertThat(Timestamp.parseTimestamp("2015-10-12T15:14:54.0"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0));
assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.123456789").getNanos())
.isEqualTo(123456789);
assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.000020000"))
.isEqualTo(Timestamp.ofTimeMicroseconds(20L));
assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.00002"))
.isEqualTo(Timestamp.ofTimeMicroseconds(20L));
assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.000020"))
.isEqualTo(Timestamp.ofTimeMicroseconds(20L));
assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.00012340"))
.isEqualTo(Timestamp.ofTimeSecondsAndNanos(0, 123400));
parseInvalidTimestamp("0001-01-01 00:00:00");
parseInvalidTimestamp("0001-1-1 00:00:00");
parseInvalidTimestamp("0001-01-01T00:00:00.");
parseInvalidTimestamp("0001-01-01T00:00:00.1234567890"); // too long
parseInvalidTimestamp("0001-01-01T00:00");
}

@Test
Expand Down