intel / DML

Intel® Data Mover Library (Intel® DML)
https://intel.github.io/DML/
MIT License
81 stars 17 forks source link

Excessive initialization in descriptor and completion_record #14

Open vinser52 opened 2 years ago

vinser52 commented 2 years ago

The descriptor struct contains excessive bytes array initialization in default ctor. I believe the following code can be safely removed: https://github.com/intel/DML/blob/develop/include/dml/detail/common/types.hpp#L21-L24. The bytes array is zero-initialized.

The same is also applicable for completion_record.

I am not sure about performance impact - I hope compiler will optimize it out when compiled with -O2.

EgorKupaev commented 2 years ago

Hi @vinser52, I'm not sure that the bytes array is initialized with zeroes in initialization list of a constructor. Especially when () are used. Could you point me to some reference or the standard's rule?

I'm 99% sure that the excessive initialization will be optimized away for -O2, but if there is no requirement from C++ standard to zero-initialize class' array members as if {} were used, then there might be a bug. I tried to be explicit.

vinser52 commented 2 years ago

Hi @EgorKupaev, you can refer to the following links: https://en.cppreference.com/w/cpp/language/value_initialization https://en.cppreference.com/w/cpp/language/zero_initialization

Value initialization is performed when a non-static data member or a base class is initialized using a [member initializer](https://en.cppreference.com/w/cpp/language/initializer_list) with an empty pair of parentheses or braces.
The effects of value initialization are:

1) if T is a class type with no [default constructor](https://en.cppreference.com/w/cpp/language/default_constructor) or with a user-provided or deleted [default constructor](https://en.cppreference.com/w/cpp/language/default_constructor), the object is [default-initialized](https://en.cppreference.com/w/cpp/language/default_initialization);
2) if T is a class type with a default constructor that is neither user-provided nor deleted (that is, it may be a class with an implicitly-defined or defaulted default constructor), the object is [zero-initialized](https://en.cppreference.com/w/cpp/language/zero_initialization) and the semantic constraints for default-initialization are checked, and if T has a non-trivial default constructor, the object is [default-initialized](https://en.cppreference.com/w/cpp/language/default_initialization);
3) if T is an array type, each element of the array is value-initialized;
4) otherwise, the object is [zero-initialized](https://en.cppreference.com/w/cpp/language/zero_initialization).

Also, I have created a simple test program:

#include <iostream>
#include <memory>
#include <cstring>
#include <algorithm>

using byte_t = std::uint8_t;

struct A {
    A() : bytes() {
#if 0
        for (auto& byte : bytes)
        {
            byte = 0u;
        }
#endif
    }

    byte_t bytes[64];
};

using allocator_type = std::allocator<A>;
using allocator_traits = std::allocator_traits<allocator_type>;

int main() {
  allocator_type alloc;
  A* pA = allocator_traits::allocate(alloc, 1);

  std::memset(pA, 5, sizeof(A));

  std::cout << std::all_of(pA->bytes, pA->bytes+64, [](byte_t byte) { return byte == 5; }) << std::endl;

  allocator_traits::construct(alloc, pA);

  std::cout << std::all_of(pA->bytes, pA->bytes+64, [](byte_t byte) { return byte == 0; }) << std::endl;

  return 0;
}
mzhukova commented 1 year ago

Hi @vinser52, as I can see this issue was opened a long time ago, so just checking in to see if it could be closed?

vinser52 commented 1 year ago

Hi @mzhukova, I think the issue I raised was not resolved.