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

Add new benchmarks #899

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add new benchmarks #899

wants to merge 1 commit into from

Conversation

lplewa
Copy link
Contributor

@lplewa lplewa commented Nov 14, 2024

  • L0 benchmark will be added in separate PR (unless merge of this one will take too long ;))
  • Old benchmark will be removed in separate PRs too
  • Please focus more on bechmark framework - particular benchmark will be also changed in separate PRs - current set is more a demo of framework - to select good benchmark scenarios there is litte bit more research to be done.

@lplewa lplewa force-pushed the benchmark branch 3 times, most recently from dbb4976 to 57edb2d Compare November 14, 2024 14:58
};

#if (defined UMF_BUILD_LIBUMF_POOL_DISJOINT)

Copy link
Contributor

Choose a reason for hiding this comment

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

rm empty line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return &disjoint_memory_pool_params;
}
};

Copy link
Contributor

Choose a reason for hiding this comment

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

rm empty line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}
};

#if (defined UMF_BUILD_LIBUMF_POOL_DISJOINT)
Copy link
Contributor

Choose a reason for hiding this comment

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

if single def check, #ifdef should suffice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

# Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

# In MSVC builds, there is no way to determine the actual build type during the
# CMake configuration step. Therefore, this message is printed in all MSVC
# builds.

set(CMAKE_CXX_STANDARD 17)
Copy link
Contributor

Choose a reason for hiding this comment

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

if you want to enforce cxx standard setting, pls use OPTIONS_REQUIRING_CXX variable in top-level CMake

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -1,10 +1,22 @@
# Copyright (C) 2023 Intel Corporation
# Copyright (C) 2023-2024 Intel Corporation
# Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

# In MSVC builds, there is no way to determine the actual build type during the
# CMake configuration step. Therefore, this message is printed in all MSVC
# builds.
Copy link
Contributor

Choose a reason for hiding this comment

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

heh the comment here is for the if down below, pls move it down

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

# Benchmark passes if it prints "PASSED" in the output, because ubench of
# scalable pool fails if the confidence interval exceeds maximum permitted
# 2.5%.
set_tests_properties(
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, as long as we're still using ubench the line should be unchanged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well - i whold have to do extra work to enable it only for ubench, when we will remove it few days later

@@ -117,7 +123,12 @@ endif()
add_umf_benchmark(
NAME ubench
SRCS ubench.c
LIBS ${LIBS_OPTIONAL}
LIBDIRS ${LIB_DIRS})
Copy link
Contributor

Choose a reason for hiding this comment

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

optional libs are no longer needed...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}
};

ALLOC_BENCHMARK_TEMPLATE_DEFINE(alloc_benchmark, stdmalloc, fix_alloc_size);
Copy link
Contributor

Choose a reason for hiding this comment

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

you could add some comments here; I'm assuming these are benchmarking scenarios...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep - i added comment to indicate it. 

I have plan to make it little bit better - those scenarios are more a demo then actual final benchmarks.

size_t size;
};

#define ALLOC_BENCHMARK_TEMPLATE_DEFINE(A, B, ...) \
Copy link
Contributor

Choose a reason for hiding this comment

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

what's A and B? perhaps a link to docs, here, in the comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@lplewa lplewa force-pushed the benchmark branch 2 times, most recently from a16b405 to fa7183a Compare November 14, 2024 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants