Kurt Deschler has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19875 )

Change subject: IMPALA-12134: Optimize row materialization time
......................................................................


Patch Set 1:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/19875/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19875/1//COMMIT_MSG@9
PS1, Line 9:  improves row materialization time by
> The improvements are Beeswax specific at the moment if I understand correct
Done


http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/benchmarks/date-benchmark.cc
File be/src/benchmarks/date-benchmark.cc:

http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/benchmarks/date-benchmark.cc@41
PS1, Line 41: // ToYearMonthDay:            Function  iters/ms   10%ile   
50%ile   90%ile     10%ile     50%ile     90%ile
            : //                                                                
          (relative) (relative) (relative)
            : // 
---------------------------------------------------------------------------------------------------------
            : //              TestCctzToYearMonthDay               23.1     
23.4     23.7         1X         1X         1X
            : //                  TestToYearMonthDay                 68     
69.6     70.7      2.95X      2.98X      2.98X
            : //                          TestToYear                443      
446      448      19.2X      19.1X      18.9X
            : //                        TestToString               9.02     
9.04     9.06     0.391X     0.386X     0.382X
            : //           TestToString_stringstream               2.04     
2.04     2.08    0.0883X    0.0871X    0.0875X
> Can you update this with your new results?
Done


http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/runtime/date-parse-util.cc
File be/src/runtime/date-parse-util.cc:

http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/runtime/date-parse-util.cc@134
PS1, Line 134:
> nit: extra line
Done


http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/runtime/date-value.cc
File be/src/runtime/date-value.cc:

http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/runtime/date-value.cc@431
PS1, Line 431: ToString
> ToString could also use the optimized default formatter.
Done


http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/runtime/timestamp-parse-util.cc
File be/src/runtime/timestamp-parse-util.cc:

http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/runtime/timestamp-parse-util.cc@235
PS1, Line 235: const date& d, const time_duration& t
> The year() / hours() ... etc functions will throw an exception  if the date
Format does not have any special handling. Why is this different?


http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/runtime/timestamp-parse-util.cc@236
PS1, Line 236:   ZeroPad(dst, d.year(), 4);
             :   ZeroPad(dst + 5, d.month().as_number(), 2);
             :   ZeroPad(dst + 8, d.day(), 2);
> I think that this could be faster if year/month/day were calculated in a si
Done


http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/runtime/timestamp-value.cc
File be/src/runtime/timestamp-value.cc:

http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/runtime/timestamp-value.cc@182
PS1, Line 182: operator
> ToString() could also use the optimized default formatter, see my notes in
Done


http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/runtime/timestamp-value.cc@183
PS1, Line 183: fractional_seconds
> Calling fractional_seconds() will throw an exception if the timestamp is no
Done


http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/service/query-result-set.cc
File be/src/service/query-result-set.cc:

http://gerrit.cloudera.org:8080/#/c/19875/1/be/src/service/query-result-set.cc@96
PS1, Line 96: medata
> typo
Done



--
To view, visit http://gerrit.cloudera.org:8080/19875
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ef5e4137fa6c2d0a5f08b430e01e3fb7de86330
Gerrit-Change-Number: 19875
Gerrit-PatchSet: 1
Gerrit-Owner: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Comment-Date: Mon, 15 May 2023 18:53:25 +0000
Gerrit-HasComments: Yes

Reply via email to