Dispatcher for template parameters of BuildHist Kernels

[razdoburdin]

Hi, I continue working on optimizations of histogram building. In this PR I suggest to refactor dispatching of runtime flags into the template parameters.
Currently histogram building functions require 3-4 template parameters. In the next optimization step few more template parameters will required. It will make template parameters list not a good looking. Moreover it will require more dispatcher functions.
For this reasons, I suggest to use a special dispatcher class, being described in this PR.

[RAMitchell]

Does std::variant do what you want? We don't have C++17 yet but we could either upgrade or just implement a common::variant and switch it to the standard library when we do upgrade.

[razdoburdin]

I don't think so.
std::variant will allow to store objects of various classes (various classes from GHistBuildingManager template, in my case). But here are at least two problems:

1. One needs to manually determine possible classes for std:variant (std::variant<Class1, Class2, ..., ClassN> var). Number of such classes grows exponentially with increasing the number of template parameters.
2. One still needs some way to convert runtime variables to specific class of GHistBuildingManager template. But if such a way exist, it is easier to use this class as a template parameter.

Did I catch your idea correctly?

[RAMitchell]

More specifically std::variant in conjunction with std::visit performs dispatching like you have in your code:

GHistBuildingManager<any_missing>::DispatchAndExecute(
    {first_page, read_by_column || force_read_by_column, bin_type_size},
    [&](auto t) {
      using BuildingManager = decltype(t);
      BuildHistDispatch<BuildingManager>(gpair, row_indices, gmat, hist);
    });

[razdoburdin]

You are right, this part is quite similar to std::visit.
However, one still need to write something like:

std::variant<GHistBuildingManager<any_missing, true,  true,  uint8_t>,
              GHistBuildingManager<any_missing, true,  true,  uint16_t>,
              GHistBuildingManager<any_missing, true,  true,  uint32_t>,
              GHistBuildingManager<any_missing, true,  false, uint8_t>,
              GHistBuildingManager<any_missing, true,  false, uint16_t>,
              GHistBuildingManager<any_missing, true,  false, uint32_t>,
              GHistBuildingManager<any_missing, false, true,  uint8_t>,
              GHistBuildingManager<any_missing, false, true,  uint16_t>,
              GHistBuildingManager<any_missing, false, true,  uint32_t>,
              GHistBuildingManager<any_missing, false, false, uint8_t>,
              GHistBuildingManager<any_missing, false, false, uint16_t>,
              GHistBuildingManager<any_missing, false, false, uint32_t>> building_manager;

to define std::variant. And amount of template parameters here grow twice if someone will add one more flag.
Also initialization of this std::variant could be quite tricky.

Do you see something here, that I have missed?

[RAMitchell]

Here is how we can instantiate many booleans into template functions without writing too much code:

In your case you have the enum as well, so you can wrap SomeFunction inside the dispatcher for this enum.

#include <iostream>
#include <variant>

std::variant<std::false_type,std::true_type> to_variant(bool b)
{
    if(b){
        return std::true_type{};
    }else{
        return std::false_type{};
    }
}

template <bool is_missing, bool something_else>
void SomeFunction(){
    std::cout << is_missing << std::endl;
    std::cout << something_else << std::endl;
}

int main()
{
    bool runtime_a = true;
    bool runtime_b = false;
    std::visit([](auto a, auto b) {
            SomeFunction<a, b>();
        }, to_variant(runtime_a), to_variant(runtime_b));

}

Given that we don't have C++17 I leave it up to you what you want to do.

[razdoburdin]

Got it! The solution based on std::visit is really much less verbose. But adding of a common::variant can be quite tricky and take a time. Can the current realization be added first? One can change it to std::variant after the C++17 will be supported.

[razdoburdin]

@RAMitchell What is your decision about this PR?

[razdoburdin]

Hi @trivialfis,
could you please describe the merging process?
This PR is already approved but still not merged. Are there any actions required from my side to speed up the process?

[hcho3]

Merging this, since all CPU tests passed.