anko-intel commented on a change in pull request #20412:
URL: https://github.com/apache/incubator-mxnet/pull/20412#discussion_r754144419
##########
File path: include/mxnet/base.h
##########
@@ -541,7 +541,11 @@ inline std::ostream& operator<<(std::ostream &out, const
Context &ctx) {
#if MXNET_USE_ONEDNN == 1 || MXNET_USE_INTGEMM == 1
-constexpr size_t kDNNLAlign = 64;
+#ifdef __linux__
+constexpr size_t kDNNLAlign = 1 << 21;
Review comment:
is there any system value we can use as huge page size?
If not you can define earlier gHugePage2MB here.
##########
File path: src/common/utils.h
##########
@@ -983,9 +986,21 @@ inline bool AlignedMemAlloc(void** ptr, size_t size,
size_t alignment) {
if (*ptr == nullptr)
return false;
#else
- int res = posix_memalign(ptr, alignment, size);
- if (res != 0)
+ int res = posix_memalign(ptr, alignment, size);
+#if __linux__
+ constexpr size_t gHugePage2MB = 1 << 21;
+ if (size >= gHugePage2MB) {
+ // TODO(mozga-intel): Enable MacOS Huge Pages if desired
+ res = posix_memalign(ptr, gHugePage2MB, size);
+ if (res == 0) {
+ madvise(ptr, size, MADV_HUGEPAGE);
Review comment:
from manual:
Most common kernels configurations provide MADV_HUGEPAGE-
style behavior by default, and thus MADV_HUGEPAGE is
normally not necessary.
Did you check if the setting is really needed?
##########
File path: src/common/utils.h
##########
@@ -983,9 +986,21 @@ inline bool AlignedMemAlloc(void** ptr, size_t size,
size_t alignment) {
if (*ptr == nullptr)
return false;
#else
- int res = posix_memalign(ptr, alignment, size);
- if (res != 0)
+ int res = posix_memalign(ptr, alignment, size);
Review comment:
alignment here already has value (1<<21)
see src/storage/cpu_device_storage.h:55
##########
File path: include/mxnet/base.h
##########
@@ -541,7 +541,11 @@ inline std::ostream& operator<<(std::ostream &out, const
Context &ctx) {
#if MXNET_USE_ONEDNN == 1 || MXNET_USE_INTGEMM == 1
-constexpr size_t kDNNLAlign = 64;
+#ifdef __linux__
+constexpr size_t kDNNLAlign = 1 << 21;
+#else
+constexpr size_t kDNNLAlign = 4096ul;
Review comment:
I guess it is standard page size, on windows. Maybe we can use something
like
https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/ns-sysinfoapi-system_info
?
##########
File path: src/common/utils.h
##########
@@ -983,9 +986,21 @@ inline bool AlignedMemAlloc(void** ptr, size_t size,
size_t alignment) {
if (*ptr == nullptr)
return false;
#else
- int res = posix_memalign(ptr, alignment, size);
- if (res != 0)
+ int res = posix_memalign(ptr, alignment, size);
+#if __linux__
+ constexpr size_t gHugePage2MB = 1 << 21;
+ if (size >= gHugePage2MB) {
Review comment:
what about in performance point of view
```suggestion
if (size >= 2 * gHugePage2MB) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
