Opened 9 years ago
Closed 9 years ago
#13376 closed enhancement (fixed)
Use Java 8 Date API (JSR 310)?
| Reported by: | simon04 | Owned by: | team |
|---|---|---|---|
| Priority: | normal | Milestone: | 16.12 |
| Component: | Core | Version: | |
| Keywords: | java8 jsr310 performance timestamp | Cc: |
Description
I did some performance tests for timestamp parsing.
package org.openstreetmap.josm.tools.date; import java.time.Instant; import java.time.ZonedDateTime; import java.util.Random; import java.util.stream.IntStream; import java.util.stream.Stream; import org.junit.Test; public class DateUtilsPerformanceTest { Stream<String> getTestData() { final Random random = new Random(23528390L); return IntStream.generate(() -> 0) .limit(4_000_000) .mapToObj(x -> String.format("%04d-%02d-%02dT%02d:%02d:%02dZ", 2000 + random.nextInt(20), 1 + random.nextInt(12), 1 + random.nextInt(28), random.nextInt(24), random.nextInt(60), random.nextInt(60) )); } @Test public void testDateUtils() throws Exception { getTestData().forEach(DateUtils::fromString); } @Test public void testZonedDateTime() throws Exception { getTestData().forEach(ZonedDateTime::parse); } @Test public void testInstant() throws Exception { getTestData().forEach(Instant::parse); } }
The results:
| DateUtils.parse using Calendar.set (as is now) | 14s 769ms |
| DateUtils.parse using ZonedDateTime.of (see patch) | 13s 388ms |
| ZonedDateTime.parse | 26s 620ms |
| Instant.parse | 22s 222ms |
So, the current DateUtil is quite performant :).
Attachments (1)
Change History (12)
by , 9 years ago
| Attachment: | DateUtil.ZonedDateTime.patch added |
|---|
comment:1 by , 9 years ago
| Milestone: | → 16.08 |
|---|
comment:2 by , 9 years ago
| Milestone: | 16.08 → 16.09 |
|---|
comment:5 by , 9 years ago
Left to be done/discussed: replace 257 occurrences of Date by Instant, or leave it as it is :)
comment:6 by , 9 years ago
Given the number of deprecated methods in Date it's almost if the class is now deprecated itself.
I'd say we can change everything that does not impact plugin API.
comment:7 by , 9 years ago
| Milestone: | 16.09 → 16.10 |
|---|
Compatibility will is a major concern since methods like AbstractPrimitive#getTimestamp, AbstractPrimitive#setTimestamp, Note#getCreatedAt, Note#getClosedAt all take/return Dates. So changing that is definitively nothing for the upcoming release. :)
comment:11 by , 9 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
New ticket for remaining point: #14176



Go ahead :)