UCP: Implementation of routine to query datatype attributes

[rdietric]

What

Implementation of the the routine ucp_dt_query according to PR #8120. Currently, the only datatype attribute to query is the packed size.

Why ?

The size of a message or data transfer is a performance-relevant information. Performance tools should be able to query this information similar to MPI_Type_size to query the size of any MPI datatype

How ?

For contig data types, the return value of ucp_contig_dt_elem_size() provides the element size. For generic data types, a field packed_size has been added to ucp_dt_generic_t, which is set with the user-defined pack routine. When the size of the generic data type is queried, the value of this field is used. All other data types are not supported and UCS_ERR_INVALID_PARAM is returned as ucs_status_t.

A unit tests verifies the expected behavior for contig, iov and generic data types. It is also checked that UCS_ERR_UNSUPPORTED is returned, if the generic datatype is queried before packing.

[yosefe]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 3 pipeline(s).

[rdietric]

Since nobody reviewed yet, I rebased to have the new ucp.h with ucp_dt_query in the branch. I also fixed a bug in the implementation and another issue in the unit test. The current pipeline failures seem unrelated to this PR.

[brminich]

@rakhmets, can you please review?

[yosefe]

LGTM besides :

• API doc review by @tonycurtis
• Set default value of count to 1 and use it for contig types
• Fix CI failures

[yosefe]

@tonycurtis @shamisp can you pls review?

[rdietric]

I squashed the commits and all checks have passed.

[yosefe]

👍 @tonycurtis @shamisp can you pls review the API change?

[tonycurtis]

On Sep 4, 2022, at 11:29 AM, Yossi Itigin @.***> wrote:

@yosefe commented on this pull request.

In src/ucp/api/ucp.h https://github.com/openucx/ucx/pull/8150#discussion_r962332275:

  */

• size_t packed_size;

• const void *buffer;
• /**

• * Number of elements in @a buffer.
  

• * This value is optional.
  

• * If @ref UCP_DATATYPE_ATTR_FIELD_COUNT is not set in @ref field_mask, the
  

• * value of this field defaults to 1.
  

@tonycurtis https://github.com/tonycurtis @shamisp https://github.com/shamisp if the default is 0, it means that if this parameter is not supplied the function will always return 0. and that is essentially as if we're making this parameter mandatory. IMO it should be 1 by default since the neutral value w.r.t. multiplication is 1.

A NULL pointer encapsulating data of length 1 seems weird. Maybe the semantics of this routine need to be revisited?

tony

[yosefe]

A NULL pointer encapsulating data of length 1 seems weird. Maybe the semantics of this routine need to be revisited? tony

The pointer itself is not mandatory for calculating the size. The single mandatory parameter is the datatype itself, and count=1 by default would mean to calculate the size of a single element.

[yosefe]

A NULL pointer encapsulating data of length 1 seems weird. Maybe the semantics of this routine need to be revisited? tony

The pointer itself is not mandatory for calculating the size. The single mandatory parameter is the datatype itself, and count=1 by default would mean to calculate the size of a single element.

@shamisp @tonycurtis WDYT?

[tonycurtis]

Yeah, that sounds good

On Wed, Sep 21, 2022 at 7:53 PM Pavel Shamis (Pasha) < @.***> wrote:

@.**** commented on this pull request.

In src/ucp/api/ucp.h https://github.com/openucx/ucx/pull/8150#discussion_r977070472:

  */

• size_t packed_size;

• const void *buffer;
• /**

• * Number of elements in @a buffer.
  

• * This value is optional.
  

• * If @ref UCP_DATATYPE_ATTR_FIELD_COUNT is not set in @ref field_mask, the
  

• * value of this field defaults to 1.
  

If @ref UCP_DATATYPE_ATTR_FIELD_COUNT is not set in @ref field_mask, the function executes the query for a single ucp_datatype_t . I think this sounds better. @tonycurtis https://github.com/tonycurtis ?

— Reply to this email directly, view it on GitHub https://github.com/openucx/ucx/pull/8150#discussion_r977070472, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABB6C2CHAD6ILYZGNKVOGRDV7ONXBANCNFSM5UB5KY4A . You are receiving this because you were mentioned.Message ID: @.***>