openPMD / openPMD/openPMD-api

Performance: Attribute reading in the HDF5 backend is implemented inefficiently

Open
#1,639 0 comments 0 reactions 0 assignees View on GitHub
backend: HDF5
Dominant language
C++
Stars
161
Forks
59
Avg merge
2d 22h
Merged PRs (30d)
7

Description

This issue should serve as documentation of a performance issue seen in our HDF5 backend.

**Describe the issue**

When parsing an HDF5 dataset, the majority of time will be spent reading attributes. The following graph was created by using `openpmd-ls` on a PIConGPU output with ~80 iterations:
![grafik](https://github.com/openPMD/openPMD-api/assets/14241876/fbfa53fc-d2fc-4c3b-9606-2cff1fd55eab)

Looking into the [implementation](https://github.com/openPMD/openPMD-api/blob/a2071184cfae03854939c8d72d865f492b6a2b5e/src/IO/HDF5/HDF5IOHandler.cpp#L2095) of `HDF5IOHandlerImpl::readAttribute()`:

```cpp
void HDF5IOHandlerImpl::readAttribute(
Writable *writable, Parameter ¶meters)
{
/* [ ... ] */

auto res = getFile(writable);
File file = res ? res.value() : getFile(writable->parent).value();

/* [ ... ] */

hid_t fapl = H5Pcreate(H5P_LINK_ACCESS);

/* [ ... ] */

obj_id =
H5Oopen(file.id, concrete_h5_file_position(writable).c_str(), fapl);

/* [ ... ] */

H5S_class_t attr_class = H5Sget_simple_extent_type(attr_space);
Attribute a(0);
if (attr_class == H5S_SCALAR ||
(attr_class == H5S_SIMPLE && ndims == 1 && dims[0] == 1))
{
if (H5Tequal(attr_type, H5T_NATIVE_CHAR))
{
char c;
status = H5Aread(attr_id, attr_type, &c);
a = Attribute(c);
}
else if (H5Tequal(attr_type, H5T_NATIVE_UCHAR))
{
unsigned char u;
status = H5Aread(attr_id, attr_type, &u);
a = Attribute(u);
}

/* [ ... ] */
/* [ ... ] */
/* [ ... ] */

else
throw error::ReadError(
error::AffectedObject::Attribute,
error::Reason::UnexpectedContent,
"HDF5",
"[HDF5] Unsupported scalar attribute type for '" + attr_name +
"'.");
}
else if (attr_class == H5S_SIMPLE)
{
if (ndims != 1)
throw error::ReadError(
error::AffectedObject::Attribute,
error::Reason::UnexpectedContent,
"HDF5",
"[HDF5] Unsupported attribute (array with ndims != 1)");

if (H5Tequal(attr_type, H5T_NATIVE_CHAR))
{
std::vector vc(dims[0], 0);
status = H5Aread(attr_id, attr_type, vc.data());
a = Attribute(vc);
}
else if (H5Tequal(attr_type, H5T_NATIVE_UCHAR))
{
std::vector vu(dims[0], 0);
status = H5Aread(attr_id, attr_type, vu.data());
a = Attribute(vu);
}

/* [ ... ] */
/* [ ... ] */
/* [ ... ] */

else
{

/* [ ... ] */

}
}
else
throw std::runtime_error("[HDF5] Unsupported attribute class");

/* [ ... ] */

status = H5Tclose(attr_type);
/* [ ... ] */
status = H5Sclose(attr_space);
/* [ ... ] */

auto dtype = parameters.dtype;
*dtype = a.dtype;
auto resource = parameters.resource;
*resource = a.getResource();

status = H5Aclose(attr_id);
/* [ ... ] */
status = H5Oclose(obj_id);
/* [ ... ] */
status = H5Pclose(fapl);
/* [ ... ] */
}
```

This exposes several problems:

1. For each read attribute, the object containing the attribute is opened and then closed. For subsequent attribute reads at the same object, this is unnecessary.
2. For each read attribute, a property is created and then closed. This is generally unnecessary, the property should be stored at the IOHandler level.
3. I've seen benchmarks before (though not the one above) where `H5Tequal()` was a sizeable portion of runtime. I'm not sure if we can find a better way to detect the attribute type.

**To Reproduce**
Not needed as the cause of the inefficiency is understood.

**Expected behavior**
Would a `READ_ATTRIBUTES` task help that reads all attributes in a group? --> Not fully since the frontend explicitly reads some attributes. Buffer the attributes of the last active group?
Definitely buffer the attribute read property.

**Additional context**
Not labeling as a a bug, since this is inefficient, but neither pathetically nor surprisingly so.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.