Skip to content

Commit 70999a3

Browse files
eregoneregon
committed
[GR-19220] [GR-31846] Fix formatting of microseconds in Time#strftime's fast-path (#2370)
PullRequest: truffleruby/2695
2 parents f80ebb8 + 38b09dc commit 70999a3

File tree

2 files changed

+47
-35
lines changed

2 files changed

+47
-35
lines changed

‎src/main/java/org/truffleruby/core/time/RubyDateFormatter.java

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -566,19 +566,20 @@ public static RopeBuilder formatToRopeBuilder(Token[] compiledPattern, ZonedDate
566566
return toAppendTo;
567567
}
568568

569-
public static boolean formatToRopeBuilderCanBeFast(Token[] compiledPattern) {
569+
@TruffleBoundary
570+
public static boolean formatCanBeFast(Token[] compiledPattern) {
570571
for (int i = 0, compiledPatternLength = compiledPattern.length; i < compiledPatternLength; i++) {
571572
Token token = compiledPattern[i];
572573
Format format = token.getFormat();
573574

574575
switch (format) {
575576
case FORMAT_ENCODING:
576-
// We only care about UTF8 encoding
577+
// Only handle UTF-8 for fast formats
577578
if (token.getData() != UTF8Encoding.INSTANCE) {
578579
return false;
579580
}
580581
break;
581-
case FORMAT_OUTPUT:
582+
case FORMAT_OUTPUT: // only %6N (optimizing for Logger::Formatter default format)
582583
RubyTimeOutputFormatter formatter = (RubyTimeOutputFormatter) token.getData();
583584

584585
// Check for the attributes present in the default case
@@ -594,14 +595,19 @@ public static boolean formatToRopeBuilderCanBeFast(Token[] compiledPattern) {
594595
return false;
595596
}
596597
break;
598+
case FORMAT_NANOSEC: // only %6N (optimizing for Logger::Formatter default format)
599+
if (i - 1 >= 0 && compiledPattern[i - 1].getFormat() == Format.FORMAT_OUTPUT) {
600+
break;
601+
} else {
602+
return false;
603+
}
597604
case FORMAT_STRING:
598605
case FORMAT_DAY:
599606
case FORMAT_HOUR:
600607
case FORMAT_MINUTES:
601608
case FORMAT_MONTH:
602609
case FORMAT_SECONDS:
603610
case FORMAT_YEAR_LONG:
604-
case FORMAT_NANOSEC:
605611
break;
606612
default:
607613
return false;
@@ -612,8 +618,8 @@ public static boolean formatToRopeBuilderCanBeFast(Token[] compiledPattern) {
612618
}
613619

614620
@ExplodeLoop
615-
public static Rope formatToRopeBuilderFast(Token[] compiledPattern, ZonedDateTime dt,
616-
RopeNodes.ConcatNode concatNode, RopeNodes.SubstringNode substringNode) {
621+
public static Rope formatToRopeFast(Token[] compiledPattern, ZonedDateTime dt,
622+
RopeNodes.ConcatNode concatNode) {
617623
Rope rope = null;
618624

619625
for (Token token : compiledPattern) {
@@ -645,28 +651,32 @@ public static Rope formatToRopeBuilderFast(Token[] compiledPattern, ZonedDateTim
645651

646652
case FORMAT_YEAR_LONG: {
647653
final int value = dt.getYear();
648-
649654
assert value >= 1000;
650655
assert value <= 9999;
651656

652657
appendRope = new LazyIntRope(value, UTF8Encoding.INSTANCE, 4);
653658
}
654659
break;
655660

656-
case FORMAT_NANOSEC: {
661+
case FORMAT_NANOSEC: { // always %6N, checked by formatCanBeFast()
657662
final int nano = dt.getNano();
658-
final LazyIntRope nanoRope = new LazyIntRope(nano);
663+
final LazyIntRope microSecondRope = new LazyIntRope(nano / 1000);
659664

665+
// This fast-path only handles the '%6N' format, so output will always be 6 characters long.
660666
final int length = 6;
661-
final int padding = length - nanoRope.characterLength();
667+
final int padding = length - microSecondRope.characterLength();
662668

669+
// `padding` is guaranteed to be >= 0 because `nano` can be at most 9 digits long before the
670+
// conversion to microseconds. The division further constrains the rope to be at most 6 digits long.
671+
assert padding >= 0 : microSecondRope;
663672
if (padding == 0) {
664-
appendRope = nanoRope;
665-
} else if (padding < 0) {
666-
appendRope = substringNode.executeSubstring(nanoRope, 0, length);
673+
appendRope = microSecondRope;
667674
} else {
668675
appendRope = concatNode
669-
.executeConcat(nanoRope, RopeConstants.paddingZeros(padding), UTF8Encoding.INSTANCE);
676+
.executeConcat(
677+
RopeConstants.paddingZeros(padding),
678+
microSecondRope,
679+
UTF8Encoding.INSTANCE);
670680
}
671681
}
672682
break;
@@ -682,6 +692,10 @@ public static Rope formatToRopeBuilderFast(Token[] compiledPattern, ZonedDateTim
682692
}
683693
}
684694

695+
if (rope == null) {
696+
rope = RopeConstants.EMPTY_UTF8_ROPE;
697+
}
698+
685699
return rope;
686700
}
687701

‎src/main/java/org/truffleruby/core/time/TimeNodes.java

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import com.oracle.truffle.api.dsl.Cached;
1515
import com.oracle.truffle.api.dsl.ImportStatic;
1616
import com.oracle.truffle.api.dsl.Specialization;
17-
import com.oracle.truffle.api.frame.VirtualFrame;
1817
import com.oracle.truffle.api.library.CachedLibrary;
1918
import com.oracle.truffle.api.object.Shape;
2019
import com.oracle.truffle.api.profiles.ConditionProfile;
@@ -374,28 +373,36 @@ public abstract static class TimeStrftimePrimitiveNode extends PrimitiveArrayArg
374373
@Specialization(
375374
guards = { "equalNode.execute(libFormat.getRope(format), cachedFormat)" },
376375
limit = "getLanguage().options.TIME_FORMAT_CACHE")
377-
protected RubyString timeStrftime(VirtualFrame frame, RubyTime time, Object format,
376+
protected RubyString timeStrftime(RubyTime time, Object format,
378377
@CachedLibrary(limit = "2") RubyStringLibrary libFormat,
379378
@Cached("libFormat.getRope(format)") Rope cachedFormat,
380379
@Cached(value = "compilePattern(cachedFormat)", dimensions = 1) Token[] pattern,
381380
@Cached RopeNodes.EqualNode equalNode,
382-
@Cached("formatToRopeBuilderCanBeFast(pattern)") boolean canUseFast,
381+
@Cached("formatCanBeFast(pattern)") boolean canUseFast,
383382
@Cached ConditionProfile yearIsFastProfile,
384-
@Cached RopeNodes.ConcatNode concatNode,
385-
@Cached RopeNodes.SubstringNode substringNode) {
383+
@Cached RopeNodes.ConcatNode concatNode) {
386384
if (canUseFast && yearIsFastProfile.profile(yearIsFast(time))) {
387-
return makeStringNode.fromRope(RubyDateFormatter.formatToRopeBuilderFast(
388-
pattern,
389-
time.dateTime,
390-
concatNode,
391-
substringNode));
385+
return makeStringNode.fromRope(RubyDateFormatter.formatToRopeFast(pattern, time.dateTime, concatNode));
392386
} else {
393387
return makeStringNode.fromBuilderUnsafe(formatTime(time, pattern), CodeRange.CR_UNKNOWN);
394388
}
395389
}
396390

397-
protected boolean formatToRopeBuilderCanBeFast(Token[] pattern) {
398-
return RubyDateFormatter.formatToRopeBuilderCanBeFast(pattern);
391+
@TruffleBoundary
392+
@Specialization(guards = "libFormat.isRubyString(format)")
393+
protected RubyString timeStrftime(RubyTime time, Object format,
394+
@CachedLibrary(limit = "2") RubyStringLibrary libFormat,
395+
@Cached RopeNodes.ConcatNode concatNode) {
396+
final Token[] pattern = compilePattern(libFormat.getRope(format));
397+
if (formatCanBeFast(pattern) && yearIsFast(time)) {
398+
return makeStringNode.fromRope(RubyDateFormatter.formatToRopeFast(pattern, time.dateTime, concatNode));
399+
} else {
400+
return makeStringNode.fromBuilderUnsafe(formatTime(time, pattern), CodeRange.CR_UNKNOWN);
401+
}
402+
}
403+
404+
protected boolean formatCanBeFast(Token[] pattern) {
405+
return RubyDateFormatter.formatCanBeFast(pattern);
399406
}
400407

401408
protected boolean yearIsFast(RubyTime time) {
@@ -404,15 +411,6 @@ protected boolean yearIsFast(RubyTime time) {
404411
return year >= 1000 && year <= 9999;
405412
}
406413

407-
@Specialization(guards = "libFormat.isRubyString(format)")
408-
protected RubyString timeStrftime(VirtualFrame frame, RubyTime time, Object format,
409-
@CachedLibrary(limit = "2") RubyStringLibrary libFormat) {
410-
final Token[] pattern = compilePattern(libFormat.getRope(format));
411-
return makeStringNode.fromBuilderUnsafe(
412-
formatTime(time, pattern),
413-
CodeRange.CR_UNKNOWN);
414-
}
415-
416414
protected Token[] compilePattern(Rope format) {
417415
return RubyDateFormatter.compilePattern(format, false, getContext(), this);
418416
}

0 commit comments

Comments
 (0)