Optimize `convertToLocalDateTime` method

[leonchen83]

speed up convertToLocalDateTime method more details see benchmark following.

Signed-off-by: Baoyi Chen [email protected]

[leonchen83]

Benchmark

import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.concurrent.TimeUnit;

import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;
import org.openjdk.jmh.runner.Runner;
import org.openjdk.jmh.runner.options.Options;
import org.openjdk.jmh.runner.options.OptionsBuilder;

/**
 * @author Baoyi Chen
 */
@Fork(1)
@State(Scope.Benchmark)
@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS)
@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
public class ConvertLocalDateTimeBenchmark {
	
	private static final DateTimeFormatter FORMATTER_SECONDS = DateTimeFormatter.ofPattern("yyyyMMdd-HH:mm:ss");
	private static final DateTimeFormatter FORMATTER_MILLIS  = DateTimeFormatter.ofPattern("yyyyMMdd-HH:mm:ss.SSS");
	private static final DateTimeFormatter FORMATTER_MICROS  = DateTimeFormatter.ofPattern("yyyyMMdd-HH:mm:ss.SSSSSS");
	private static final DateTimeFormatter FORMATTER_NANOS   = DateTimeFormatter.ofPattern("yyyyMMdd-HH:mm:ss.SSSSSSSSS");
	
	public static void main(String[] args) throws Exception {
		Options opt = new OptionsBuilder()
				.shouldDoGC(true)
				.include(ConvertLocalDateTimeBenchmark.class.getSimpleName())
				.build();
		new Runner(opt).run();
	}
	
	@Benchmark
	public LocalDateTime benchmarkRaw() {
		return convertToLocalDateTimeRaw("20120922-12:34:56.123456789");
	}
	
	@Benchmark
	public LocalDateTime benchmarkNew() {
		return convertToLocalDateTimeNew("20120922-12:34:56.123456789");
	}
	
	public static LocalDateTime convertToLocalDateTimeNew(String value) {
		int length = value.length();
		int ns = 0;
		if (length >= 27) {
			ns = parseInt(value, 18, 9);
		} else if (length == 24) {
			ns = parseInt(value, 18, 6) * 1000;
		} else if (length == 21) {
			ns = parseInt(value, 18, 3) * 1000000;
		}
		
		int yy = parseInt(value, 0, 4);
		int mm = parseInt(value, 4, 2);
		int dd = parseInt(value, 6, 2);
		int h = parseInt(value, 9, 2);
		int m = parseInt(value, 12, 2);
		int s = parseInt(value, 15, 2);
		return LocalDateTime.of(yy, mm, dd, h, m, s, ns);
	}
	
	private static int parseInt(String value, int off, int len) {
		int num = 0;
		for (int index = 0; index < len; index++) {
			num = (num * 10) + value.charAt(off + index) - '0';
		}
		return num;
	}
	
	public static LocalDateTime convertToLocalDateTimeRaw(String value) {
		int length = value.length();
		switch (length) {
			case 17:
				return LocalDateTime.parse(value, FORMATTER_SECONDS);
			case 21:
				return LocalDateTime.parse(value, FORMATTER_MILLIS);
			case 24:
				return LocalDateTime.parse(value, FORMATTER_MICROS);
			case 27:
			case 30:
				return LocalDateTime.parse(value.substring(0, 27), FORMATTER_NANOS);
			default:
				throw new UnsupportedOperationException();
		}
	}
}

Benchmark result

Benchmark                                    Mode  Cnt         Score         Error  Units
ConvertLocalDateTimeBenchmark.benchmarkNew  thrpt    5  34913770.478 ± 3381097.452  ops/s
ConvertLocalDateTimeBenchmark.benchmarkRaw  thrpt    5   1335947.490 ±   72909.323  ops/s

30 times faster than before

[chrjohn]

Thanks @leonchen83 , I'll have a look.

[raipc]

@chrjohn @leonchen83 I think that decrementing the leap second is not the best solution since we'll have time going downward. Would moving to the next second with zero nanos be sufficient?

[chrjohn]

Hi @raipc @leonchen83 @philipwhiuk

sorry for the delay. So it seems we have two options if we don't want time to go backwards (related to time of the previous message):

Either store the last nanos as outlined here: https://github.com/quickfix-j/quickfixj/issues/418#issuecomment-1005331199

Or can't we (for the sake of simplicity) assume that nanos are 999999? E.g. convert 19981231-23:59:60.444444 to 19981231-23:59:59.999999

Did I miss something?

[leonchen83]

hi @chrjohn or we could just optimize convertToLocalDateTime method and ignore leap second problem. one PR fix one thing? WDYT？

[chrjohn]

Yes a separate PR would be good in any case. Thanks

[chrjohn]

Could you please open a new (or change this) PR to base this on master? If needed I could cherry pick it to a minor version later on. Thanks

[leonchen83]

hi refer to PR529

[chrjohn]

Great, thanks. Could you please open a separate PR against master with the original changes, i.e. also the leap second stuff? It should be no problem to merge it after #529 and after we finished the discussion regarding the nanos.

[leonchen83]

@chrjohn 2 commits on one PR, is that OK?

[chrjohn]

Sorry for not being clear. I meant one PR with your optimization and another one with your optimization including the leap second stuff (both against master branch). So I would be able to merge the first PR separately. Thank you again

[chrjohn]

Superseded by #529