-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
Changes from 3 commits
7bd1485
fee061b
16657ee
85aee11
acf3c11
b1feca4
be151f8
2cd776d
03384a0
82a8257
63001ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| /* | ||
| * Copyright 2019 Google LLC | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.google.cloud; | ||
|
|
||
| import com.google.common.base.Preconditions; | ||
|
|
||
| /** | ||
| * Util class for fast parsing of integer values. This is used by {@link Date#parseDate(String)} and | ||
| * {@link Timestamp#parseTimestamp(String)}. These parse methods are used internally by Google | ||
| * client libraries to parse text values returned by services, and these parse methods should be as | ||
| * efficient as possible. | ||
| */ | ||
| class IntParser { | ||
|
|
||
| private static final int[] POWERS_OF_10 = { | ||
| 1, 10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000 | ||
| }; | ||
|
|
||
| /** Parses an int from the given input between the specified begin and end. */ | ||
| static int parseInt(String input, int begin, int end) { | ||
| return parseInt(input, begin, end, 0); | ||
| } | ||
|
|
||
| /** | ||
| * Parses an int from the given input between the specified begin and end. The value is multiplied | ||
| * by 10^exponent. The exponent must be between 0 and 10 inclusive. | ||
| */ | ||
| static int parseInt(String input, int begin, int end, int exponent) { | ||
| Preconditions.checkNotNull(input); | ||
| Preconditions.checkArgument( | ||
| exponent >= 0 && exponent < POWERS_OF_10.length, "Exponent out of range"); | ||
| Preconditions.checkArgument(end - begin <= 10, "Max input length is 10"); | ||
| Preconditions.checkArgument(end >= begin, "End must be greater or equal to begin"); | ||
| Preconditions.checkArgument(begin >= 0, "Begin must be >= 0"); | ||
| Preconditions.checkArgument(end <= input.length(), "End must be <= input.length()"); | ||
| int res = 0; | ||
| for (int index = begin; index < end; index++) { | ||
| res += parseDigit(input.charAt(index), input) * POWERS_OF_10[end - index - 1]; | ||
| } | ||
| return res * POWERS_OF_10[exponent]; | ||
| } | ||
|
|
||
| private static int parseDigit(char c, String input) { | ||
| if (c == '0') { | ||
olavloite marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return 0; | ||
| } else if (c == '1') { | ||
| return 1; | ||
| } else if (c == '2') { | ||
| return 2; | ||
| } else if (c == '3') { | ||
| return 3; | ||
| } else if (c == '4') { | ||
| return 4; | ||
| } else if (c == '5') { | ||
| return 5; | ||
| } else if (c == '6') { | ||
| return 6; | ||
| } else if (c == '7') { | ||
| return 7; | ||
| } else if (c == '8') { | ||
| return 8; | ||
| } else if (c == '9') { | ||
| return 9; | ||
| } else { | ||
| throw new NumberFormatException("Not a digit: " + c); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
|
|
@@ -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; | ||
|
|
||
|
|
@@ -181,9 +170,46 @@ public com.google.protobuf.Timestamp toProto() { | |
| * 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', invalidTimestamp); | ||
| Preconditions.checkArgument( | ||
| (timestamp.length() == 19 && timestamp.charAt(18) != 'Z') | ||
| || (timestamp.length() == 20 && timestamp.charAt(19) == 'Z') | ||
| || (timestamp.charAt(19) == '.' | ||
| && timestamp.length() > 20 | ||
| && timestamp.charAt(20) != 'Z'), | ||
| invalidTimestamp); | ||
| int year = IntParser.parseInt(timestamp, 0, 4); | ||
| int month = IntParser.parseInt(timestamp, 5, 7); | ||
| int day = IntParser.parseInt(timestamp, 8, 10); | ||
| int hour = IntParser.parseInt(timestamp, 11, 13); | ||
| int minute = IntParser.parseInt(timestamp, 14, 16); | ||
| int second = IntParser.parseInt(timestamp, 17, 19); | ||
| int fraction = 0; | ||
| if (timestamp.length() > 20) { | ||
| if (timestamp.charAt(19) == '.') { | ||
| int endIndex; | ||
| if (timestamp.charAt(timestamp.length() - 1) == 'Z') { | ||
| endIndex = timestamp.length() - 1; | ||
| } else { | ||
|
||
| endIndex = timestamp.length(); | ||
| } | ||
| if (endIndex - 20 > 9) { | ||
| throw new IllegalArgumentException(invalidTimestamp); | ||
| } | ||
| // Adjust the result to nanoseconds if the input length is less than 9 digits (9 - (endIndex | ||
| // - 20)). | ||
| fraction = IntParser.parseInt(timestamp, 20, endIndex, 9 - (endIndex - 20)); | ||
| } else { | ||
| } | ||
| } | ||
| LocalDateTime ldt = LocalDateTime.of(year, month, day, hour, minute, second, fraction); | ||
| return ofTimeSecondsAndNanos(ldt.toEpochSecond(ZoneOffset.UTC), ldt.getNano()); | ||
| } | ||
|
|
||
| private StringBuilder toString(StringBuilder b) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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.text.ParseException; | ||
|
|
@@ -38,6 +39,20 @@ public void parseDate() { | |
| assertThat(date.getYear()).isEqualTo(2016); | ||
| assertThat(date.getMonth()).isEqualTo(9); | ||
| assertThat(date.getDayOfMonth()).isEqualTo(18); | ||
| parseInvalidDate("2016/09/18"); | ||
| parseInvalidDate("2016 09 18"); | ||
| parseInvalidDate("2016-9-18"); | ||
| parseInvalidDate("2016-09-18T10:00"); | ||
| parseInvalidDate(""); | ||
|
||
| } | ||
|
|
||
| private void parseInvalidDate(String input) { | ||
| try { | ||
| Date.parseDate(input); | ||
| fail("Expected exception"); | ||
| } catch (IllegalArgumentException e) { | ||
| assertThat(e.getMessage()).contains("Invalid date"); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| /* | ||
| * Copyright 2019 Google LLC | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.google.cloud; | ||
|
|
||
| import static com.google.common.truth.Truth.assertThat; | ||
| import static org.junit.Assert.fail; | ||
|
|
||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
| import org.junit.runners.JUnit4; | ||
|
|
||
| @RunWith(JUnit4.class) | ||
| public class IntParserTest { | ||
|
|
||
| @Test | ||
| public void testParse() { | ||
| assertThat(IntParser.parseInt("1", 0, 1)).isEqualTo(1); | ||
| assertThat(IntParser.parseInt("1234", 0, 4)).isEqualTo(1234); | ||
| assertThat(IntParser.parseInt("1234", 2, 4)).isEqualTo(34); | ||
| assertThat(IntParser.parseInt("1234", 0, 2)).isEqualTo(12); | ||
| assertThat(IntParser.parseInt("1234567890", 0, 10)).isEqualTo(1234567890); | ||
| assertThat(IntParser.parseInt("1234567890", 1, 10)).isEqualTo(234567890); | ||
| assertThat(IntParser.parseInt("0123456789", 0, 10)).isEqualTo(123456789); | ||
| assertThat(IntParser.parseInt("00001234", 0, 8)).isEqualTo(1234); | ||
| assertThat(IntParser.parseInt("", 0, 0)).isEqualTo(0); | ||
| parseInvalidNumber("test", 0, 4); | ||
| parseInvalidNumber("123T456", 0, 4); | ||
| parseInvalidArgument("", 0, 1); | ||
| parseInvalidArgument("1234", 0, 5); | ||
| parseInvalidArgument("1234", -1, 4); | ||
| parseInvalidArgument("1234", 3, 2); | ||
| } | ||
|
|
||
| private void parseInvalidNumber(String input, int begin, int end) { | ||
| try { | ||
| IntParser.parseInt(input, begin, end); | ||
| fail("Expected exception"); | ||
| } catch (NumberFormatException e) { | ||
| } | ||
| } | ||
|
|
||
| private void parseInvalidArgument(String input, int begin, int end) { | ||
| try { | ||
| IntParser.parseInt(input, begin, end); | ||
| fail("Expected exception"); | ||
| } catch (IllegalArgumentException e) { | ||
| } | ||
| } | ||
| } |
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.
This change seems straightforward. Can you please break it out into a separate PR?
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.
Removed from this PR and submitted as separate PR.