Skip to content

Reduce tensorprotoutils binary size #6634

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.

Sign up for GitHub

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

Merged
skottmckay merged 7 commits into from
Feb 12, 2021
Merged

Reduce tensorprotoutils binary size #6634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3eb6673
Move type agnostic code out of UnpackInitializerData
skottmckay Feb 9, 2021
46dde76
Remove templatization of more parts
skottmckay Feb 10, 2021
cc9bc00
Fix build warning by moving a couple of things around
skottmckay Feb 10, 2021
eb0474a
Address PR comments
skottmckay Feb 10, 2021
059350e
Fix linux build error due to no auto conversion for SafeInt there.
skottmckay Feb 11, 2021
82da124
Merge remote-tracking branch 'origin/master' into skottmckay/ReduceTe…
skottmckay Feb 12, 2021
7c26a37
Address PR comments.
skottmckay Feb 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 22 additions & 12 deletions onnxruntime/core/framework/endian_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ OutputIt ReverseCopy(BidirIt first, BidirIt last, OutputIt d_first) {

} // namespace

void SwapByteOrderCopy(
size_t element_size_in_bytes,
gsl::span<const char> source_bytes, gsl::span<char> destination_bytes) {
void SwapByteOrderCopy(size_t element_size_in_bytes,
gsl::span<const unsigned char> source_bytes,
gsl::span<unsigned char> destination_bytes) {
assert(element_size_in_bytes > 0);
assert(source_bytes.size_bytes() % element_size_in_bytes == 0);
assert(source_bytes.size_bytes() == destination_bytes.size_bytes());
Expand All @@ -40,28 +40,38 @@ void SwapByteOrderCopy(
for (size_t element_offset = 0, element_offset_end = source_bytes.size_bytes();
element_offset < element_offset_end;
element_offset += element_size_in_bytes) {
const auto source_element_bytes =
source_bytes.subspan(element_offset, element_size_in_bytes);
const auto dest_element_bytes =
destination_bytes.subspan(element_offset, element_size_in_bytes);
ReverseCopy(
source_element_bytes.data(),
source_element_bytes.data() + source_element_bytes.size_bytes(),
dest_element_bytes.data());
const auto source_element_bytes = source_bytes.subspan(element_offset, element_size_in_bytes);
const auto dest_element_bytes = destination_bytes.subspan(element_offset, element_size_in_bytes);
ReverseCopy(source_element_bytes.data(),
source_element_bytes.data() + source_element_bytes.size_bytes(),
dest_element_bytes.data());
}
}

namespace detail {

void CopyLittleEndian(size_t element_size_in_bytes, gsl::span<const char> source_bytes, gsl::span<char> destination_bytes) {
Status CopyLittleEndian(size_t element_size_in_bytes,
gsl::span<const unsigned char> source_bytes,
gsl::span<unsigned char> destination_bytes) {
ORT_RETURN_IF(source_bytes.size_bytes() != destination_bytes.size_bytes(),
"source and destination buffer size mismatch");

if (endian::native == endian::little) {
std::memcpy(destination_bytes.data(), source_bytes.data(), source_bytes.size_bytes());
} else {
SwapByteOrderCopy(element_size_in_bytes, source_bytes, destination_bytes);
}

return Status::OK();
}

} // namespace detail

common::Status ReadLittleEndian(size_t element_size,
gsl::span<const unsigned char> source_bytes,
gsl::span<unsigned char> destination_bytes) {
return detail::CopyLittleEndian(element_size, source_bytes, destination_bytes);
}

} // namespace utils
} // namespace onnxruntime
39 changes: 21 additions & 18 deletions onnxruntime/core/framework/endian_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,52 +27,55 @@ namespace utils {
* @param source_bytes The source byte span.
* @param destination_bytes The destination byte span.
*/
void SwapByteOrderCopy(
size_t element_size_in_bytes, gsl::span<const char> source_bytes, gsl::span<char> destination_bytes);
void SwapByteOrderCopy(size_t element_size_in_bytes,
gsl::span<const unsigned char> source_bytes,
gsl::span<unsigned char> destination_bytes);

namespace detail {

/**
* Copies between two buffers where one is little-endian and the other has
* native endian-ness.
*/
void CopyLittleEndian(
size_t element_size_in_bytes, gsl::span<const char> source_bytes, gsl::span<char> destination_bytes);
Status CopyLittleEndian(size_t element_size_in_bytes,
gsl::span<const unsigned char> source_bytes,
gsl::span<unsigned char> destination_bytes);

} // namespace detail

/**
* Reads from a little-endian source.
*/
common::Status ReadLittleEndian(size_t element_size,
gsl::span<const unsigned char> source_bytes,
gsl::span<unsigned char> destination_bytes);

/**
* Reads from a little-endian source with check that T is trivially copyable.
Copy link
Contributor

@edgchen1 edgchen1 Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: only true if not GCC below 5

Copy link
Contributor

@edgchen1 edgchen1 Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we are stuck with supporting the older GCC versions, maybe we can consider this:
https://stackoverflow.com/a/31798726

Copy link
Contributor Author

@skottmckay skottmckay Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure that matters. If there's code that is going to fail, it will fail in our CIs which use later versions of GCC won't it, as we don't have any code that is selectively included for a build with GCC < 5?

Copy link
Contributor

@edgchen1 edgchen1 Feb 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, then my original comment applies, regarding "with check that T is trivially copyable."

* @remarks Check is skipped for if building with gcc v4
*/
template <typename T>
common::Status ReadLittleEndian(gsl::span<const char> source_bytes, gsl::span<T> destination) {
common::Status ReadLittleEndian(gsl::span<const unsigned char> source_bytes, gsl::span<T> destination) {
// std::is_trivially_copyable is not implemented in older versions of GCC
#if !defined(__GNUC__) || __GNUC__ >= 5
static_assert(std::is_trivially_copyable<T>::value, "T must be trivially copyable");
#endif
ORT_RETURN_IF_NOT(source_bytes.size_bytes() == destination.size_bytes(),
"source and destination buffer size mismatch");
const auto destination_bytes = gsl::make_span(
reinterpret_cast<char*>(destination.data()), destination.size_bytes());
detail::CopyLittleEndian(sizeof(T), source_bytes, destination_bytes);
return common::Status::OK();
const auto destination_bytes = gsl::make_span(reinterpret_cast<unsigned char*>(destination.data()),
destination.size_bytes());
return ReadLittleEndian(sizeof(T), source_bytes, destination_bytes);
}

/**
* Writes to a little-endian destination.
*/
template <typename T>
common::Status WriteLittleEndian(gsl::span<const T> source, gsl::span<char> destination_bytes) {
common::Status WriteLittleEndian(gsl::span<const T> source, gsl::span<unsigned char> destination_bytes) {
// std::is_trivially_copyable is not implemented in older versions of GCC
#if !defined(__GNUC__) || __GNUC__ >= 5
static_assert(std::is_trivially_copyable<T>::value, "T must be trivially copyable");
#endif
ORT_RETURN_IF_NOT(source.size_bytes() == destination_bytes.size_bytes(),
"source and destination buffer size mismatch");
const auto source_bytes = gsl::make_span(
reinterpret_cast<const char*>(source.data()), source.size_bytes());
detail::CopyLittleEndian(sizeof(T), source_bytes, destination_bytes);
return common::Status::OK();
const auto source_bytes = gsl::make_span(reinterpret_cast<const unsigned char*>(source.data()), source.size_bytes());
return detail::CopyLittleEndian(sizeof(T), source_bytes, destination_bytes);
}

} // namespace utils
Expand Down
Loading