[BUG] parquet reader::impl::decode_page_data error_code checking slowness

[abellina]

While benchmarking some code we found that about 5% worth of time are being lost due to this line of code:
https://github.com/rapidsai/cudf/blob/branch-24.04/cpp/src/io/parquet/reader_impl.cpp#L248

This is on our perf cluster (A100) for NDS @3k. It explains some of a dip in perf we have seen since 23.10 but we haven't gotten around to testing.

If I stop obtaining the value from error_code (e.g. I don't perform the pageable memcpy essentially) we gain 20 seconds locally. I am filing this because it may be a good idea to remove this or look into how to improve it (would a pinned copy help)?

[abellina]

@nvdbaranec @vuule fyi

[vuule]

That's wild. Have you maybe looked at a profile to see how we spend the additional time?

[abellina]

it's over many queries, I think it is due to the pageable copy (synchronization due to staging copies). I wonder if a quick prototype would be to swap the pageable copy by a pinned one and see if we recover the amount of time lost.

[vuule]

I can prototype something tomorrow. My concern is that we'll break even without the pinned pool due to the extra pinned allocation.

[abellina]

this would imply having @nvdbaranec's pinned pool #15079, so the allocation should be virtually free :)

[vuule]

yeah but only spark will have the pool for now ;)

[GregoryKimball]

Thank you @abellina for posting this. Please also consult #14167 and #14415 for a similar error code check and performance fix. Would you please investigate a single-GPU single-node reproducer for this performance issue? Do you think a single-GPU reproducer is possible?

[abellina]

@nvdbaranec @vuule fyi

[vuule]

That's wild. Have you maybe looked at a profile to see how we spend the additional time?

[abellina]

it's over many queries, I think it is due to the pageable copy (synchronization due to staging copies). I wonder if a quick prototype would be to swap the pageable copy by a pinned one and see if we recover the amount of time lost.

[vuule]

I can prototype something tomorrow. My concern is that we'll break even without the pinned pool due to the extra pinned allocation.

[abellina]

this would imply having @nvdbaranec's pinned pool #15079, so the allocation should be virtually free :)

[vuule]

yeah but only spark will have the pool for now ;)

[GregoryKimball]

Thank you @abellina for posting this. Please also consult #14167 and #14415 for a similar error code check and performance fix. Would you please investigate a single-GPU single-node reproducer for this performance issue? Do you think a single-GPU reproducer is possible?