GH-39357: [C++] Reduce function.h includes

[zanmato1984]

Rationale for this change

As proposed in #36246 , by splitting function option structs from function.h, we can reduce the including of function.h. So that the total build time could be reduced.

The total parser time could be reduced from 722.3s to 709.7s. And the function.h along with its transitive inclusion of kernel.h don't show up in expensive headers any more.

The detailed analysis result before and after this PR are attached:
analyze-before.txt
analyze-after.txt

Disclaimer (quote from #36246 (comment)):

Note that the time diff is not absolute. The ClangBuildAnalyzer result differs from time to time. I guess it depends on the idle-ness of the building machine when doing the experiment. But the time reduction is almost certain, though sometimes more sometimes less. And the inclusion times of the questioning headers are reduced for sure, as shown in the attachments in my other comment.

What changes are included in this PR?

Move function option structs into own compute/options.h, and change including function.h to including options.h wherever fits.

Are these changes tested?

Build is testing.

Are there any user-facing changes?

There could be potential build failures for user code (quote from #36246 (comment)):

The header function.h remains in compute/api.h, with and without this PR. The proposed PR removes function.h from api_xxx.h (then includes options.h instead), as proposed in the initial description of this issue. This results in compile failures for user code which includes only compute/api_xxx.h but not compute/api.h, and meanwhile uses CallFunction which is declared in function.h.

But I think it's OK as described in #36246 (comment).

• Closes: [C++] Includings of function.h are expensive #39357

[github-actions]

⚠️ GitHub issue #36246 has been automatically assigned in GitHub to PR creator.

[github-actions]

⚠️ GitHub issue #39357 has been automatically assigned in GitHub to PR creator.

[zanmato1984]

cc @pitrou @felipecrv

[felipecrv]

Very nice improvement! Only two comments about options.h.

[felipecrv]

A better name for this header is function_options.h to communicate better what the options are about and have it close to function.h when both are included.

[felipecrv]

And since api.h is a catch-all include for end-user's convenience, we should have #include "arrow/compute/function_options.h" in there.

[zanmato1984]

A better name for this header is function_options.h to communicate better what the options are about and have it close to function.h when both are included.

Sure, will do.

And since api.h is a catch-all include for end-user's convenience, we should have #include "arrow/compute/function_options.h" in there.

Did you mean in compute/api.h? If so, it's already there (of course I will change the file name).

Or in api.h of arrow root include? If so, I don't think we should include compute stuff in there.

Would you please help to confirm? Appreciate it.

[zanmato1984]

function.h renamed to function_options.h.

[felipecrv]

Sorry. I meant compute/api.h and for some reason I didn't notice the include was already there.

[felipecrv]

What this means here?

Oh. I see it now. There is a comment like this in function.h as well :D

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit cf44793.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.