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

[CORE] OV file utils file_size use std file path #28462

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
887dd2b
move wide character string conversions to separate file
barnasm1 Jan 15, 2025
04671c7
refactor
barnasm1 Jan 15, 2025
b334528
refactor file size functions to use ov::util::Path and handle Android…
barnasm1 Jan 15, 2025
c0ec93e
add unit tests for cut_android_path function
barnasm1 Jan 15, 2025
dce5d59
add unit tests for file size functions with various file types and en…
barnasm1 Jan 15, 2025
701cd96
code format
barnasm1 Jan 15, 2025
97f2a96
prevent variable redefinition for android
barnasm1 Jan 15, 2025
c0345b1
refactor test to use ov::util::Path for Unicode file output
barnasm1 Jan 15, 2025
66615dc
Merge branch 'master' into ov_file_path_util_file_size
mlukasze Jan 16, 2025
a1e8053
add Windows-specific includes for wstring_cast_util
barnasm1 Jan 16, 2025
f34b2d2
fix comment typo
barnasm1 Jan 16, 2025
2d071bf
refactor cpp_version.hpp and file_path.hpp for compiler version checks
barnasm1 Jan 16, 2025
3b2fbae
refactor WPath function to use lambda syntax, to meet Naming style check
barnasm1 Jan 16, 2025
eed127e
update copyright year to 2025, fix typos,
barnasm1 Jan 16, 2025
067224b
rename wstring_cast_util to wstring_convert_util and update includes
barnasm1 Jan 16, 2025
22d6712
add Android-specific file size test
barnasm1 Jan 16, 2025
2e7cf52
fix typo in CMakeLists.txt for wstring_convert_util source file path
barnasm1 Jan 16, 2025
e9c2cca
deprecate old file_size function and provide alternative usage guidance
barnasm1 Jan 16, 2025
71e2206
update file size tests to use string literals
barnasm1 Jan 16, 2025
c8bdb3b
skip file size tests based on C++ version
barnasm1 Jan 16, 2025
dd13c04
revert: refactor WPath function to use lambda syntax, to meet Naming …
barnasm1 Jan 16, 2025
74640d8
Merge branch 'master' into ov_file_path_util_file_size
barnasm1 Jan 21, 2025
0f2aa4a
clang format
barnasm1 Jan 22, 2025
76d0a49
Merge branch 'master' into ov_file_path_util_file_size
barnasm1 Jan 23, 2025
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
1 change: 1 addition & 0 deletions src/common/util/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ endif()

set_source_files_properties(
"${CMAKE_CURRENT_SOURCE_DIR}/src/file_util.cpp"
"${CMAKE_CURRENT_SOURCE_DIR}/src/wstring_cast_util.cpp"
PROPERTIES COMPILE_DEFINITIONS OpenVINO_VERSION="${OpenVINO_VERSION}")

source_group("src" FILES ${LIBRARY_SRC})
Expand Down
16 changes: 16 additions & 0 deletions src/common/util/include/openvino/util/cpp_version.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,19 @@
# endif
# endif
#endif

#if !defined(__GNUC__) || (__GNUC__ > 12 || __GNUC__ == 12 && __GNUC_MINOR__ >= 3)
# define GCC_NOT_USED_OR_VER_AT_LEAST_12_3
#endif

#if !defined(__clang__) || defined(__clang__) && __clang_major__ >= 17
# define CLANG_NOT_USED_OR_VER_AT_LEAST_17
#endif

#if defined(__GNUC__) && (__GNUC__ < 12 || __GNUC__ == 12 && __GNUC_MINOR__ < 3)
# define GCC_VER_LESS_THEN_12_3
barnasm1 marked this conversation as resolved.
Show resolved Hide resolved
barnasm1 marked this conversation as resolved.
Show resolved Hide resolved
#endif

#if defined(__clang__) && __clang_major__ < 17
# define CLANG_VER_LESS_THEN_17
barnasm1 marked this conversation as resolved.
Show resolved Hide resolved
#endif
barnasm1 marked this conversation as resolved.
Show resolved Hide resolved
17 changes: 17 additions & 0 deletions src/common/util/include/openvino/util/file_path.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,17 @@
#include <cstdio>

#include "openvino/util/filesystem.hpp"
#include "openvino/util/wstring_cast_util.hpp"

namespace ov {
namespace util {

#if defined(OPENVINO_HAS_FILESYSTEM)
// There are known issues related with usage of std::filesystem::path unocode represenataion:
barnasm1 marked this conversation as resolved.
Show resolved Hide resolved
barnasm1 marked this conversation as resolved.
Show resolved Hide resolved
// https://jira.devtools.intel.com/browse/CVS-160477
barnasm1 marked this conversation as resolved.
Show resolved Hide resolved
// * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95048
// * https://stackoverflow.com/questions/58521857/cross-platform-way-to-handle-stdstring-stdwstring-with-stdfilesystempath
// Working compiler versions has been designated with godbolt.
using Path = std::filesystem::path;
#elif defined(OPENVINO_HAS_EXP_FILESYSTEM)
// Known issues:
Expand All @@ -30,5 +37,15 @@ using Path = std::filesystem::path;
using Path = std::experimental::filesystem::path;
#endif

#if defined(GCC_VER_LESS_THEN_12_3) || defined(CLANG_VER_LESS_THEN_17)
barnasm1 marked this conversation as resolved.
Show resolved Hide resolved
inline ov::util::Path WPath(const std::wstring& wpath) {
return {ov::util::wstring_to_string(wpath)};
}
#else
inline ov::util::Path WPath(const std::wstring& wpath) {
return {wpath};
}
#endif

} // namespace util
} // namespace ov
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#if defined(GCC_VER_LESS_THEN_12_3) || defined(CLANG_VER_LESS_THEN_17)
inline ov::util::Path WPath(const std::wstring& wpath) {
return {ov::util::wstring_to_string(wpath)};
}
#else
inline ov::util::Path WPath(const std::wstring& wpath) {
return {wpath};
}
#endif
} // namespace util
} // namespace ov
} // namespace util
} // namespace ov
#if defined(GCC_VER_LESS_THEN_12_3) || defined(CLANG_VER_LESS_THEN_17)
template<>
ov::util::Path::path<std::wstring>(const std::wstring& source, format fmt) : path(ov::util::wstring_to_string(source), std::move(fmt)) {}
#endif

Consider add specialization for compiler version where is bug with path creation then WPath function is not required and path can be create always in same way.
Optionally: If any issue because specialization already defined add some trait check to detect it and avoid define it in OV.

59 changes: 23 additions & 36 deletions src/common/util/include/openvino/util/file_util.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
#include <string>
#include <vector>

#include "openvino/util/file_path.hpp"
#include "openvino/util/util.hpp"
#include "openvino/util/wstring_cast_util.hpp"

namespace ov {
namespace util {
Expand Down Expand Up @@ -89,19 +91,6 @@ const std::string& path_to_string(const Path& path) {
}

#ifdef OPENVINO_ENABLE_UNICODE_PATH_SUPPORT
/**
* @brief Conversion from wide character string to a single-byte chain.
* @param wstr A wide-char string
* @return A multi-byte string
*/
std::string wstring_to_string(const std::wstring& wstr);
/**
* @brief Conversion from single-byte chain to wide character string.
* @param str A null-terminated string
* @return A wide-char string
*/
std::wstring string_to_wstring(const std::string& str);

/**
* @brief Convert path as wide character string to a single-byte chain.
* @param path Path as wide-char string.
Expand Down Expand Up @@ -172,26 +161,30 @@ bool directory_exists(const std::string& path);
bool directory_exists(const std::wstring& path);
#endif

inline ov::util::Path cut_android_path(const ov::util::Path& file_name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is not required can be still internal part of file_size

const auto& file_name_result = file_name.native();
const auto pos = file_name_result.find('!');
if (pos != std::decay_t<decltype(file_name_result)>::npos) {
return ov::util::Path(file_name_result.substr(0, pos));
}

return file_name;
Copy link
Contributor

Choose a reason for hiding this comment

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

you probably return the wrong variable (file_name instead of file_name_result)

}

/**
* @brief Returns file size for file
* @param[in] path The file name
* @return file size
*/
inline int64_t file_size(const char* path) {
barnasm1 marked this conversation as resolved.
Show resolved Hide resolved
#if defined(OPENVINO_ENABLE_UNICODE_PATH_SUPPORT) && defined(_WIN32)
std::wstring widefilename = ov::util::string_to_wstring(path);
const wchar_t* file_name = widefilename.c_str();
#elif defined(__ANDROID__) || defined(ANDROID)
std::string file_name = path;
std::string::size_type pos = file_name.find('!');
if (pos != std::string::npos) {
file_name = file_name.substr(0, pos);
}
inline int64_t file_size(const ov::util::Path& path) {
Copy link
Contributor

@praasz praasz Jan 16, 2025

Choose a reason for hiding this comment

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

Suggested change
inline int64_t file_size(const ov::util::Path& path) {
inline int64_t file_size(const ov::util::Path& path) {
#if defined(__ANDROID__) || defined(ANDROID)
// apply cuting if std::filesytem not support it well.
#endif
return std::filesystem::file_size(path);
}

This function should probably be wrapper to std implementation if works.

#if defined(__ANDROID__) || defined(ANDROID)
const ov::util::Path& cut_path = cut_android_path(path);
std::ifstream in(cut_path, std::ios_base::binary | std::ios_base::ate);
return in.tellg();
Comment on lines +201 to +202
Copy link
Contributor

Choose a reason for hiding this comment

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

Formally tellg returns current position in stream and nothing more. It does not guarantee that this value is equal to the file size.
https://en.cppreference.com/w/cpp/io/basic_istream/tellg

#else
const char* file_name = path;
#endif
std::ifstream in(file_name, std::ios_base::binary | std::ios_base::ate);
std::ifstream in(path, std::ios_base::binary | std::ios_base::ate);
return in.tellg();
#endif
}

/**
Expand Down Expand Up @@ -223,8 +216,11 @@ inline bool file_exists(const char* path) {
* @param[in] path The file name
* @return file size
*/
inline int64_t file_size(const wchar_t* path) {
return file_size(wstring_to_string(path));
}
inline int64_t file_size(const std::wstring& path) {
return file_size(wstring_to_string(path).c_str());
return file_size(wstring_to_string(path));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return file_size(wstring_to_string(path));
return file_size(Path(path));

}

/**
Expand All @@ -237,15 +233,6 @@ inline bool file_exists(const std::wstring& path) {
}
#endif // OPENVINO_ENABLE_UNICODE_PATH_SUPPORT

/**
* @brief Returns file size for file
* @param[in] path The file name
* @return file size
*/
inline int64_t file_size(const std::string& path) {
Copy link
Contributor

@praasz praasz Jan 16, 2025

Choose a reason for hiding this comment

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

Why is removed?
Should be marked deprecated first before we remove it, with information that will be removed in e.g. 2026.0

Consider add deprecation note to other function which are not using Path type.

return file_size(path.c_str());
}

/**
* @brief Returns true if file exists
* @param[in] path The file name
Expand Down
31 changes: 31 additions & 0 deletions src/common/util/include/openvino/util/wstring_cast_util.hpp
Copy link
Contributor

Choose a reason for hiding this comment

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

These functions are not cast rename file or keep them in original header.
Why these function are moved to new file?

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright (C) 2018-2024 Intel Corporation
barnasm1 marked this conversation as resolved.
Show resolved Hide resolved
// SPDX-License-Identifier: Apache-2.0
//

#pragma once

#include <string>

#include "openvino/util/util.hpp"

namespace ov {
namespace util {
barnasm1 marked this conversation as resolved.
Show resolved Hide resolved

#ifdef OPENVINO_ENABLE_UNICODE_PATH_SUPPORT
/**
* @brief Conversion from wide character string to a single-byte chain.
* @param wstr A wide-char string
* @return A multi-byte string
*/
std::string wstring_to_string(const std::wstring& wstr);
/**
* @brief Conversion from single-byte chain to wide character string.
* @param str A null-terminated string
* @return A wide-char string
*/
std::wstring string_to_wstring(const std::string& str);

#endif

} // namespace util
} // namespace ov
40 changes: 0 additions & 40 deletions src/common/util/src/file_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,46 +338,6 @@ void ov::util::convert_path_win_style(std::string& path) {
std::replace(path.begin(), path.end(), '/', '\\');
}

#ifdef OPENVINO_ENABLE_UNICODE_PATH_SUPPORT

# ifdef __clang__
# pragma clang diagnostic push
# pragma clang diagnostic ignored "-Wdeprecated-declarations"
# endif

std::string ov::util::wstring_to_string(const std::wstring& wstr) {
# ifdef _WIN32
int size_needed = WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), NULL, 0, NULL, NULL);
std::string strTo(size_needed, 0);
WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), &strTo[0], size_needed, NULL, NULL);
return strTo;
# else
std::wstring_convert<std::codecvt_utf8<wchar_t>> wstring_decoder;
return wstring_decoder.to_bytes(wstr);
# endif
}

std::wstring ov::util::string_to_wstring(const std::string& string) {
const char* str = string.c_str();
# ifdef _WIN32
int strSize = static_cast<int>(std::strlen(str));
int size_needed = MultiByteToWideChar(CP_UTF8, 0, str, strSize, NULL, 0);
std::wstring wstrTo(size_needed, 0);
MultiByteToWideChar(CP_UTF8, 0, str, strSize, &wstrTo[0], size_needed);
return wstrTo;
# else
std::wstring_convert<std::codecvt_utf8<wchar_t>> wstring_encoder;
std::wstring result = wstring_encoder.from_bytes(str);
return result;
# endif
}

# ifdef __clang__
# pragma clang diagnostic pop
# endif

#endif // OPENVINO_ENABLE_UNICODE_PATH_SUPPORT

std::string ov::util::get_absolute_file_path(const std::string& path) {
std::string absolutePath;
absolutePath.resize(MAX_ABS_PATH);
Expand Down
48 changes: 48 additions & 0 deletions src/common/util/src/wstring_cast_util.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (C) 2018-2025 Intel Corporation
// SPDX-License-Identifier: Apache-2.0
//

#include "openvino/util/wstring_cast_util.hpp"

#ifdef OPENVINO_ENABLE_UNICODE_PATH_SUPPORT

# include <codecvt>
# include <locale>

# ifdef __clang__
# pragma clang diagnostic push
# pragma clang diagnostic ignored "-Wdeprecated-declarations"
# endif

std::string ov::util::wstring_to_string(const std::wstring& wstr) {
# ifdef _WIN32
int size_needed = WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), NULL, 0, NULL, NULL);
std::string strTo(size_needed, 0);
WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), &strTo[0], size_needed, NULL, NULL);
return strTo;
# else
std::wstring_convert<std::codecvt_utf8<wchar_t>> wstring_decoder;
return wstring_decoder.to_bytes(wstr);
# endif
}

std::wstring ov::util::string_to_wstring(const std::string& string) {
const char* str = string.c_str();
# ifdef _WIN32
int strSize = static_cast<int>(std::strlen(str));
int size_needed = MultiByteToWideChar(CP_UTF8, 0, str, strSize, NULL, 0);
std::wstring wstrTo(size_needed, 0);
MultiByteToWideChar(CP_UTF8, 0, str, strSize, &wstrTo[0], size_needed);
return wstrTo;
# else
std::wstring_convert<std::codecvt_utf8<wchar_t>> wstring_encoder;
std::wstring result = wstring_encoder.from_bytes(str);
return result;
# endif
}

# ifdef __clang__
# pragma clang diagnostic pop
# endif

#endif // OPENVINO_ENABLE_UNICODE_PATH_SUPPORT
Loading
Loading