Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22293 )
Change subject: IMPALA-13627: Handle legacy Hive timezone conversion ...................................................................... Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/22293/11/be/src/benchmarks/convert-timestamp-benchmark.cc File be/src/benchmarks/convert-timestamp-benchmark.cc: http://gerrit.cloudera.org:8080/#/c/22293/11/be/src/benchmarks/convert-timestamp-benchmark.cc@115 PS11, Line 115: FromUnixTimeNanos: Function iters/ms 10%ile 50%ile 90%ile 10%ile 50%ile 90%ile : (relative) (relative) (relative) : --------------------------------------------------------------------------------------------------------- : (sec split (old)) 31.2 32 32.4 1X 1X 1X : (sec split (new)) 16.3 16.5 16.6 0.523X 0.517X 0.513X : : FromSubsecondUnixTime: Function iters/ms 10%ile 50%ile 90%ile 10%ile 50%ile 90%ile : (relative) (relative) (relative) : --------------------------------------------------------------------------------------------------------- : (old) 32 32.5 33 1X 1X 1X : (new) 14.7 14.9 15.1 0.46X 0.458X 0.458X > That's a really good question. Most other numbers improved. I'll look into IMPALA-9385 changed the conditional logic in TimestampValue::FromUnixTime, and it looks like that negated most of the benefit seen in this benchmark, presumably because branch prediction was no longer as reliable. It's no longer a fair benchmark, because the (old) implementation didn't get changes to support timezone conversion. When I change it back to TimestampValue TimestampValue::FromUnixTime(time_t unix_time, const Timezone* local_tz) { if (FLAGS_use_local_tz_for_unix_timestamp_conversions) { return UnixTimeToLocal(unix_time, *local_tz); } else { return UtcFromUnixTimeTicks<1>(unix_time); } } I get FromUnixTimeNanos: Function iters/ms 10%ile 50%ile 90%ile 10%ile 50%ile 90%ile (relative) (relative) (relative) --------------------------------------------------------------------------------------------------------- (sec split (old)) 33.9 34.5 35.1 1X 1X 1X (sec split (new)) 241 241 242 7.1X 7X 6.91X FromSubsecondUnixTime: Function iters/ms 10%ile 50%ile 90%ile 10%ile 50%ile 90%ile (relative) (relative) (relative) --------------------------------------------------------------------------------------------------------- (old) 32.7 33.4 33.8 1X 1X 1X (new) 146 147 148 4.46X 4.4X 4.39X -- To view, visit http://gerrit.cloudera.org:8080/22293 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1271ed1da0b74366ab8315e7ec2d4ee47111e067 Gerrit-Change-Number: 22293 Gerrit-PatchSet: 12 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Wed, 12 Feb 2025 00:01:10 +0000 Gerrit-HasComments: Yes
