Use the loaded headers from the previous file

[thomasfrosio]

Instead of preprocessing every source file from scratch, why not use the already loaded headers?

Please correct me if I'm wrong.

Currently, jitify loads and preprocesses the source files in a distributed approach, i.e. each file is preprocessed independently from the others, and the loaded headers are then added to the all_header_sources map. std::unordered_map::insert() will automatically filter redundant entries on a first-in first-out basis.

This is all fine, but since the files are preprocessed one after the other, why not instead add the already known headers from the previous files to the list of headers that nvrtc can use for the next file? As I'm sure you've noticed, letting jitify collecting the headers from nvrtc's error messages quickly adds up when there's a lot of headers. For files that include a lot of the same headers (which I assume is a very common scenario), this simple change makes a big difference performance-wise and should produce exactly the same output, right? I use --shared-headers option and it seems to work fine.

HTH,
Thomas

[thomasfrosio]

After looking at it more closely, this may be dangerous with the --cuda-std option on.
In this scenario, passing <cuda/std/*> headers in header_sources will call the patch_cuda_source function on these headers, which I'm pretty sure, is not something we want.

This happens here:

jitify/jitify2.hpp

Lines 5973 to 5983 in 2a7d754

	for (auto& name_source : header_sources) {
	const std::string& header_name = name_source.first;
	std::string& header_source = name_source.second;
	bool is_jitify_preinclude = header_name == "jitify_preinclude.h";
	bool is_cuda_std_header =
	detail::get_workaround_system_headers().count(header_name);
	header_source = detail::patch_cuda_source(
	header_source,
	use_cuda_std && !is_jitify_preinclude && !is_cuda_std_header,
	replace_pragma_once);
	}

These two lines seem problematic, or at least I don't understand it:

jitify/jitify2.hpp

Lines 5977 to 5978 in 2a7d754

	bool is_cuda_std_header =
	detail::get_workaround_system_headers().count(header_name);

Is that supposed to detect <cuda/std/*> headers?

However, when loading new headers, everything seem okay and <cuda/std/*> headers are not patched thanks to these lines:

jitify/jitify2.hpp

Lines 6119 to 6123 in 2a7d754

	bool is_cuda_std_header =
	header_fullpath.find(detail::kJitifyBuiltinHeaderPrefix) == 0 ||
	// TODO: More robust way to detect this?
	header_fullpath.find(detail::path_join(
	detail::path_join("cuda", "std"), "")) != std::string::npos;

Not sure if I'm misusing the library or if it's a bug.

[benbarsdell]

I like this idea, but I think you're right that we need to be careful to avoid patching cuda/std headers or double-patching other headers.

bool is_cuda_std_header = 
     detail::get_workaround_system_headers().count(header_name);

I think the logic here is correct (because the workaround system headers are added via header_sources), but the variable name is wrong (I probably made a copy-paste mistake). If cuda/std headers are also passed via header_sources then we will need to somehow detect them here too. Double-patching is also a potential problem, though it may already be idempotent so it wouldn't really matter besides the performance impact.

Maybe you could make this op-in via a command line flag like --experimental-reuse-headers or similar, and also only enable it if --cuda-std is not enabled.

Note that #131 will dramatically speed up preprocessing (orthogonal to this PR) once it lands. I will think about how we can enhance the new preprocessing logic to detect headers that we don't want to (re-)patch.