[VL] Add timing for velox shuffle reader#3254
Conversation
|
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/oap-project/gluten/issues Then could you also rename commit message and pull request title in the following format? See also: |
|
@kecookier Could you finish this PR according to the comment of |
| return ipcTime; | ||
| } | ||
|
|
||
| public void SetDeserializeTime(long ipcTime) { |
There was a problem hiding this comment.
SetDeserializeTime -> setDeserializeTime
There was a problem hiding this comment.
SetDeserializeTime -> setDeserializeTime
changed
| "decompressTime" -> SQLMetrics.createNanoTimingMetric(sparkContext, "totaltime_decompress"), | ||
| "ipcTime" -> SQLMetrics.createNanoTimingMetric(sparkContext, "totaltime_ipc"), | ||
| "deserializeTime" -> SQLMetrics.createNanoTimingMetric(sparkContext, "totaltime_deserialize"), |
There was a problem hiding this comment.
Should we use the consistent metric name format?
e.g. totaltime_XXX or totaltime XXX
cc @rui-mo
There was a problem hiding this comment.
it's good to rename to totaltime XXX consistently.
There was a problem hiding this comment.
Yes, we'd better keep them aligned.
|
@kecookier @zuochunwei Would you remove "WIP" from PR title? |
Done |
zuochunwei
changed the title
zhztheplayer
changed the title
|
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
5 participants
What changes were proposed in this pull request?
Some jobs of ours consume most of their time for the operator of
ColumnarExchange, and the metrics of VeloxShuffleReader don't containIO(IPC read)anddeserializesteps.So I proposed this PR to complement these missing metrics.
(Please fill in changes proposed in this fix)
(Fixes: #ISSUE-ID)
How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)