RFC-0025 Improving incremental builds

[peterbell10]

This RFC proposes changes to ATen that will allow more granular header dependencies and tools to enforce their usage across the codebase which should greatly improve incremental and cached build performance.

Note that a lot of this has already been implemented and merged but this RFC should provide a complete picture of the motivation and how these PRs fit together.

• The stack starting with Factor out TensorBase that doesn't depend on native operators pytorch#63612 introduces TensorBase and makes various CUDA files TORCH_ASSERT_NO_OPERATORS compliant, as does Remove native_functions.yaml dependency from ScanKernels.cu pytorch#66620.
• The stack starting with CMake: Support dynamic codegen outputs pytorch#68246 generates the per-operator headers and makes various code-generated files TORCH_ASSERT_ONLY_METHOD_OPERATORS compliant.
• The unmerged stack starting with Remove CUDA Foreach... files dependency on function operators pytorch#68462 converts most of ATen to use per-operator headers.

Rendered version: https://github.com/pytorch/rfcs/blob/rfc-improve-build-times/RFC-0025-improving-incremental-builds.md

[albanD]

Given that most of the above is done today. Do we want to extend this rfc to try and answer such questions?

[albanD]

I could imagine a daily/weekly automatic CI that automatically runs/fix this similar to how we do for not important python lint with pytorch/pytorch@1edf6f5

Do you think that could work? Any danger of BC-breaking change for c++ users?

[peterbell10]

iwyu does have a "safe mode" that never remove includes from header files so I think it's possible without BC-breaking changes. The main worries are that it sometimes requires manual input like telling it which headers are public or private (e.g. List-inl.h is private and List.h is public) and also additional in-source pragmas (e.g. to allow List.h to include List-inl.h).

I think the ATen/ops headers are fairly safe though so it may be possible to filter the iwyu output to only change operator headers.

[albanD]

Some of our users include internal file and rely on the fact that most of ATen is available at that point.
This series of change (by cleaning core) will break them. Is there anything we can do to reduce such breakage while still fixing our build?

[peterbell10]

I suppose we could do something like

#ifndef CAFFE2_BUILD_MAIN_LIB
#include <ATen/ATen.h>
#endif

However, I think that user code should really just be including ATen.h itself.

[albanD]

I agree that the fix is really on the other side there.

[albanD]

I think we could answer this by seeing how much benefit we will get from doing that? Is there an easy way to see how this will change the metrics discussed above?

[peterbell10]

I don't know of an easy way but some numbers to put it into perspective: 1184 files include Tensor.h today which is ~70% of the compile time for ATen and torch. ~40% of operators have some method variant so keeping it this way means very roughly speaking we can expect ~60% of operator changes to have fast incremental builds.

However, I suspect that methods change less often and that more development effort will be going into new operators which are mostly functional-only. So in practice it may be much more the 60% that are fast.

[albanD]

Sounds great!

[albanD]

The CUDA builds are not mentioned in this doc. Does it need any special casing?

[peterbell10]

CUDA builds don't require any additional work for the main changes here. .cu files can use the per-operator headers as they would any other header and TensorBase was used first and foremost for cuda files.

One consideration is that include-what-you-use doesn't support cuda files, so using per-operator headers takes a little more manual work.