[Enhancement] Improve the randomness of the random function #35199

zenoyang · 2023-11-16T07:05:02Z

Why I'm doing:
The original random function of SR was implemented through the rand_r function. The randomness was very poor and usually did not meet expectations.

before:

after:

What I'm doing:

Fixes #34026

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function

Bugfix cherry-pick branch check:

CLAassistant · 2023-11-16T07:05:43Z

All committers have signed the CLA.

starrocks-cr · 2023-11-16T07:05:48Z

be/src/exprs/math_functions.cpp

-    context->set_function_state(FunctionContext::THREAD_LOCAL, state);
+    for (int i = 0; i < num_rows; ++i) {
+        result.append(distribution(*generator));
+    }

    return result.build(false);
 }


The most risky bug in this code is:
Using a reinterpret_cast<int64_t> for the pointer to std::mt19937_64 instance, which can cause undefined behavior when it tries to cast back and use it.

You can modify the code like this:

- void* state = context->get_function_state(FunctionContext::THREAD_LOCAL); + std::mt19937_64* generator = reinterpret_cast<std::mt19937_64*>(context->get_function_state(FunctionContext::THREAD_LOCAL)); ... - int64_t res = generate_randoms(&result, num_rows, reinterpret_cast<int64_t>(state)); - state = reinterpret_cast<void*>(res); // NOLINT - context->set_function_state(FunctionContext::THREAD_LOCAL, state); // The above lines are actually removed in the given code review as they are outdated logic not suitable for use with std::mt19937_64.

The rest of the changes provided here replace the deleted generate_randoms function with standard <random> library functions. These changes also introduce proper handling of memory allocation and deallocation for the std::mt19937_64 random number generator.

imay · 2023-11-16T19:32:36Z

@zenoyang
After run is slower than current, I think you pasted the wrong picture.

be/src/exprs/math_functions.cpp

be/src/exprs/math_functions.h

be/src/exprs/math_functions.cpp

stdpain · 2023-11-20T08:45:59Z

be/src/exprs/math_functions.cpp

    return Status::OK();
 }

 StatusOr<ColumnPtr> MathFunctions::rand(FunctionContext* context, const Columns& columns) {
-    int32_t num_rows = ColumnHelper::get_const_value<TYPE_INT>(columns[columns.size() - 1]);
-    void* state = context->get_function_state(FunctionContext::THREAD_LOCAL);
+    std::mt19937_64* generator =


declare static thread local here

Thanks, I modified it based on your suggestions, and the test performance is in line with expectations. Please help to review it again. @stdpain

zenoyang · 2023-11-20T10:23:55Z

@zenoyang After run is slower than current, I think you pasted the wrong picture.

No, that is the result of the intermediate state of development. The performance was not very good at the beginning.

stdpain · 2023-11-21T02:30:39Z

CI clang-format failed in

--- /home/runner/_work/starrocks/starrocks/be/src/exprs/math_functions.cpp
+++ /home/runner/_work/starrocks/starrocks/be/src/exprs/math_functions.cpp (after clang format)
/home/runner/_work/starrocks/starrocks/be/src/exprs/math_functions.cpp had clang-format style issues
@@ -34,7 +34,7 @@
 static const double MAX_EXP_PARAMETER = std::log(std::numeric_limits<double>::max());
 
 static std::uniform_real_distribution<double> distribution(0.0, 1.0);
-static thread_local std::mt1[9](https://github.com/StarRocks/starrocks/actions/runs/6928566833/job/18844627873?pr=35199#step:6:10)937_64 generator { std::random_device{}() };
+static thread_local std::mt[19](https://github.com/StarRocks/starrocks/actions/runs/6928566833/job/18844627873?pr=35199#step:6:20)937_64 generator{std::random_device{}()};
 
 // ==== basic check rules =========
 DEFINE_UNARY_FN_WITH_IMPL(NegativeCheck, value) {

stdpain · 2023-11-21T02:34:22Z

cla also need check

stdpain · 2023-11-21T02:35:49Z

sign off also need check. try to execute

git commit -s -m "[Enhancement] Improv...."

Signed-off-by: zenoyang <cookie.yz@qq.com>

zenoyang · 2023-11-21T06:37:26Z

sign off also need check. try to execute
git commit -s -m "[Enhancement] Improv...."

done

Signed-off-by: zenoyang <cookie.yz@qq.com>

wanpengfei-git · 2023-11-21T12:05:21Z

@Mergifyio backport branch-3.1

wanpengfei-git · 2023-11-21T12:05:23Z

@Mergifyio backport branch-3.0

wanpengfei-git · 2023-11-21T12:05:25Z

@Mergifyio backport branch-2.5

mergify · 2023-11-21T12:05:30Z

backport branch-3.2

✅ Backports have been created

#35509 [Enhancement] Improve the randomness of the random function (backport #35199) has been created for branch branch-3.2

mergify · 2023-11-21T12:05:33Z

backport branch-3.1

✅ Backports have been created

#35510 [Enhancement] Improve the randomness of the random function (backport #35199) has been created for branch branch-3.1 but encountered conflicts

mergify · 2023-11-21T12:05:35Z

backport branch-3.0

✅ Backports have been created

#35511 [Enhancement] Improve the randomness of the random function (backport #35199) has been created for branch branch-3.0 but encountered conflicts

mergify · 2023-11-21T12:05:36Z

backport branch-2.5

✅ Backports have been created

#35512 [Enhancement] Improve the randomness of the random function (backport #35199) has been created for branch branch-2.5 but encountered conflicts

Signed-off-by: zenoyang <cookie.yz@qq.com> (cherry picked from commit 893a701)

Signed-off-by: zenoyang <cookie.yz@qq.com> (cherry picked from commit 893a701) # Conflicts: # be/src/exprs/math_functions.cpp

Signed-off-by: zenoyang <cookie.yz@qq.com> (cherry picked from commit 893a701) # Conflicts: # be/src/exprs/vectorized/math_functions.cpp

Signed-off-by: zenoyang <cookie.yz@qq.com> (cherry picked from commit 893a701)

…s#35199) Signed-off-by: zenoyang <cookie.yz@qq.com> (cherry picked from commit 893a701)

github-actions bot added 3.2 3.1 3.0 2.5 labels Nov 16, 2023

mergify bot assigned zenoyang Nov 16, 2023

starrocks-cr bot reviewed Nov 16, 2023

View reviewed changes

zenoyang marked this pull request as draft November 16, 2023 12:43

zenoyang force-pushed the 231115_opt_rand_randomness branch 3 times, most recently from 56d5589 to 754dcff Compare November 16, 2023 13:28

stdpain reviewed Nov 20, 2023

View reviewed changes

be/src/exprs/math_functions.cpp Outdated Show resolved Hide resolved

stdpain self-assigned this Nov 20, 2023

stdpain reviewed Nov 20, 2023

View reviewed changes

be/src/exprs/math_functions.h Outdated Show resolved Hide resolved

stdpain reviewed Nov 20, 2023

View reviewed changes

be/src/exprs/math_functions.cpp Outdated Show resolved Hide resolved

stdpain reviewed Nov 20, 2023

View reviewed changes

zenoyang marked this pull request as ready for review November 20, 2023 10:22

stdpain previously approved these changes Nov 21, 2023

View reviewed changes

github-actions bot added the behavior_changed label Nov 21, 2023

zenoyang added 2 commits November 21, 2023 14:34

[Enhancement] Improve the randomness of the random function

c9e9e80

Signed-off-by: zenoyang <cookie.yz@qq.com>

Optimize performance

a586815

Signed-off-by: zenoyang <cookie.yz@qq.com>

zenoyang dismissed stdpain’s stale review via e41b43c November 21, 2023 06:36

zenoyang force-pushed the 231115_opt_rand_randomness branch from 7f387f6 to e41b43c Compare November 21, 2023 06:36

Optimize performance 2

9077bd8

Signed-off-by: zenoyang <cookie.yz@qq.com>

github-actions bot removed the 3.2 label Nov 21, 2023

github-actions bot removed the 3.1 label Nov 21, 2023

github-actions bot removed the 3.0 label Nov 21, 2023

github-actions bot removed the 2.5 label Nov 21, 2023

mergify bot pushed a commit that referenced this pull request Nov 21, 2023

[Enhancement] Improve the randomness of the random function (#35199)

3830d7f

Signed-off-by: zenoyang <cookie.yz@qq.com> (cherry picked from commit 893a701)

mergify bot mentioned this pull request Nov 21, 2023

[Enhancement] Improve the randomness of the random function (backport #35199) #35509

Merged

mergify bot pushed a commit that referenced this pull request Nov 21, 2023

[Enhancement] Improve the randomness of the random function (#35199)

41085fe

Signed-off-by: zenoyang <cookie.yz@qq.com> (cherry picked from commit 893a701) # Conflicts: # be/src/exprs/math_functions.cpp

mergify bot mentioned this pull request Nov 21, 2023

[Enhancement] Improve the randomness of the random function (backport #35199) #35510

Closed

mergify bot pushed a commit that referenced this pull request Nov 21, 2023

[Enhancement] Improve the randomness of the random function (#35199)

ccf8103

Signed-off-by: zenoyang <cookie.yz@qq.com> (cherry picked from commit 893a701) # Conflicts: # be/src/exprs/math_functions.cpp

mergify bot mentioned this pull request Nov 21, 2023

[Enhancement] Improve the randomness of the random function (backport #35199) #35511

Closed

mergify bot pushed a commit that referenced this pull request Nov 21, 2023

[Enhancement] Improve the randomness of the random function (#35199)

02ab53e

Signed-off-by: zenoyang <cookie.yz@qq.com> (cherry picked from commit 893a701) # Conflicts: # be/src/exprs/vectorized/math_functions.cpp

mergify bot mentioned this pull request Nov 21, 2023

[Enhancement] Improve the randomness of the random function (backport #35199) #35512

Closed

wanpengfei-git pushed a commit that referenced this pull request Nov 24, 2023

[Enhancement] Improve the randomness of the random function (#35199)

eb6ba95

Signed-off-by: zenoyang <cookie.yz@qq.com> (cherry picked from commit 893a701)

jaogoy removed the behavior_changed label Jan 25, 2024

github-actions bot added 3.2 3.1 3.0 2.5 labels Jan 25, 2024

github-actions bot removed 2.5 3.1 3.0 labels Feb 4, 2024

chaoyli pushed a commit to chaoyli/starrocks that referenced this pull request May 28, 2024

[Enhancement] Improve the randomness of the random function (StarRock…

a7e7683

…s#35199) Signed-off-by: zenoyang <cookie.yz@qq.com> (cherry picked from commit 893a701)

chaoyli pushed a commit to chaoyli/starrocks that referenced this pull request May 28, 2024

[Enhancement] Improve the randomness of the random function (StarRock…

2137cd5

…s#35199) Signed-off-by: zenoyang <cookie.yz@qq.com> (cherry picked from commit 893a701)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enhancement] Improve the randomness of the random function #35199

[Enhancement] Improve the randomness of the random function #35199

zenoyang commented Nov 16, 2023 •

edited by stdpain

CLAassistant commented Nov 16, 2023 •

edited

starrocks-cr bot Nov 16, 2023

imay commented Nov 16, 2023

stdpain Nov 20, 2023

zenoyang Nov 20, 2023

zenoyang commented Nov 20, 2023

stdpain commented Nov 21, 2023

stdpain commented Nov 21, 2023

stdpain commented Nov 21, 2023

zenoyang commented Nov 21, 2023 •

edited

wanpengfei-git commented Nov 21, 2023

wanpengfei-git commented Nov 21, 2023

wanpengfei-git commented Nov 21, 2023

mergify bot commented Nov 21, 2023 •

edited

mergify bot commented Nov 21, 2023 •

edited

mergify bot commented Nov 21, 2023 •

edited

mergify bot commented Nov 21, 2023 •

edited

[Enhancement] Improve the randomness of the random function #35199

[Enhancement] Improve the randomness of the random function #35199

Conversation

zenoyang commented Nov 16, 2023 • edited by stdpain

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

CLAassistant commented Nov 16, 2023 • edited

starrocks-cr bot Nov 16, 2023

Choose a reason for hiding this comment

imay commented Nov 16, 2023

stdpain Nov 20, 2023

Choose a reason for hiding this comment

zenoyang Nov 20, 2023

Choose a reason for hiding this comment

zenoyang commented Nov 20, 2023

stdpain commented Nov 21, 2023

stdpain commented Nov 21, 2023

stdpain commented Nov 21, 2023

zenoyang commented Nov 21, 2023 • edited

wanpengfei-git commented Nov 21, 2023

wanpengfei-git commented Nov 21, 2023

wanpengfei-git commented Nov 21, 2023

mergify bot commented Nov 21, 2023 • edited

✅ Backports have been created

mergify bot commented Nov 21, 2023 • edited

✅ Backports have been created

mergify bot commented Nov 21, 2023 • edited

✅ Backports have been created

mergify bot commented Nov 21, 2023 • edited

✅ Backports have been created

zenoyang commented Nov 16, 2023 •

edited by stdpain

CLAassistant commented Nov 16, 2023 •

edited

zenoyang commented Nov 21, 2023 •

edited

mergify bot commented Nov 21, 2023 •

edited

mergify bot commented Nov 21, 2023 •

edited

mergify bot commented Nov 21, 2023 •

edited

mergify bot commented Nov 21, 2023 •

edited