Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UCP/WIREUP: Reduce message size #1556

Merged
merged 1 commit into from
Jun 1, 2017

Conversation

MattBBaker
Copy link
Contributor

@MattBBaker MattBBaker commented May 31, 2017

The addition of latency overhead values in #1544 has exceeded the size limit for UGNI wireup message sizes. This patch reduces the type from double to float, bringing the wireup message size back under the limit.

@mellanox-github
Copy link
Contributor

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/1780/ for details.

@mellanox-github
Copy link
Contributor

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/3718/ for details (Mellanox internal link).

@shamisp shamisp requested a review from yosefe May 31, 2017 23:15
@shamisp shamisp added this to the v1.3 milestone May 31, 2017
@yosefe yosefe merged commit fd6637d into openucx:master Jun 1, 2017
@MattBBaker MattBBaker deleted the topic/reduce-wireup-message-size branch June 1, 2017 14:35
@alinask
Copy link
Contributor

alinask commented Jun 1, 2017

@yosefe @MattBBaker when packing the address, we take the value from iface_attr->latency.overhead and copy it to packed->lat_ovh: https://github.com/openucx/ucx/blob/master/src/ucp/wireup/address.c#L238

If packed->lat_ovh is now a float, shouldn't we change the source (iface_attr->latency.overhead) to the same type as well (for overflow cases)?

@MattBBaker
Copy link
Contributor Author

It's the same way with iface_attr->bandwidth and iface_attr->overhead. The uct_iface_attr_t is huge as far as structs go so there wouldn't be much space saved. It maybe worth considering out of 'least surprise' reasons though, not surprising people by letting them think they have a double but it gets down cast into a float on the wire.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants