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.

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

Jump to bottom

misc: Remove the unnecessary merge in addSingleGroupRawInput of VarianceAggregate #12397

Closed
wants to merge 2 commits into from
Closed

misc: Remove the unnecessary merge in addSingleGroupRawInput of VarianceAggregate #12397

wants to merge 2 commits into from

Conversation

zhli1142015
Copy link
Contributor

@zhli1142015 zhli1142015 commented Feb 20, 2025 •
edited
Loading

On lines 235 and 240, we performed an unnecessary Accumulator merge
operation. Instead, we can call updateNonNullValue(char* group, T value)
to update the inputs to the result Accumulator directly.

We noticed this issue when a Spark unit test on our side failed due
to floating-point precision differences (18.475208614068027 -> 18.475208614068023).
Upon investigation, we found that the extra merge was the root cause.
image

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 20, 2025
@netlify Netlify
Copy link

netlify bot commented Feb 20, 2025 •
edited
Loading

✅ Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit e950cd0
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/67b73a9d629fb70008fa563a

@zhli1142015
Copy link
Contributor Author

@Yuhta and @mbasmanova could you help to take a look? Thanks.

@mbasmanova mbasmanova requested a review from kagamiori February 20, 2025 12:08
@zhli1142015 zhli1142015 mentioned this pull request Feb 20, 2025
Closed
@zhli1142015 zhli1142015 marked this pull request as draft February 20, 2025 15:17
facebook-github-bot pushed a commit that referenced this pull request Feb 21, 2025
@zhli1142015 @facebook-github-bot
fix: Fix Inconsistent task execution mode error in presto agg benchma…
47bad1a 
…rk (#12400)

Summary:
When adding benchmark for #12397, I found that other benchmarks encounter the following error, and this PR will fix the issue.
```
E0220 21:58:18.611248 1141027 Exceptions.h:66] Line: /var/git/velox/velox/exec/Task.cpp:827, Function:checkExecutionMode, Expression: mode == mode_ (Serial vs. Parallel) Inconsistent task execution mode., Source: RUNTIME, ErrorCode: INVALID_STATE
terminate called after throwing an instance of 'facebook::velox::VeloxRuntimeError'
  what():  Exception: VeloxRuntimeError
Error Source: RUNTIME
Error Code: INVALID_STATE
Reason: (Serial vs. Parallel) Inconsistent task execution mode.
Retriable: False
Expression: mode == mode_
Function: checkExecutionMode
File: /var/git/velox/velox/exec/Task.cpp
Line: 827
Stack trace:
# 0  facebook::velox::VeloxException::VeloxException(char const*, unsigned long, char const*, std::basic_string_view<char, std::char_traits<char> >, std::basic_string_view<char, std::char_traits<char> >, std::basic_string_view<char, std::char_traits<char> >, std::basic_string_view<char, std::char_traits<char> >, bool, facebook::velox::VeloxException::Type, std::basic_string_view<char, std::char_traits<char> >)
# 1  void facebook::velox::detail::veloxCheckFail<facebook::velox::VeloxRuntimeError, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(facebook::velox::detail::VeloxCheckFailArgs const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
# 2  facebook::velox::exec::Task::checkExecutionMode(facebook::velox::exec::Task::ExecutionMode)
# 3  facebook::velox::exec::Task::next(folly::SemiFuture<folly::Unit>*)
# 4  (anonymous namespace)::SimpleAggregatesBenchmark::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
# 5  std::_Function_handler<folly::detail::TimeIterData (unsigned int), folly::detail::BenchmarkingState<std::chrono::_V2::system_clock>::addBenchmark<(anonymous namespace)::follyBenchmarkUnused16::{lambda(unsigned int)https://github.com/facebookincubator/velox/issues/1}&>(char const*, folly::Range<char const>, std::enable_if&&)::{lambda(unsigned int)https://github.com/facebookincubator/velox/issues/1}>::_M_invoke(std::_Any_data const&, unsigned int&&)
# 6  folly::runBenchmarkGetNSPerIteration(std::function<folly::detail::TimeIterData (unsigned int)> const&, double)
# 7  folly::(anonymous namespace)::runBenchmarksWithPrinterImpl(folly::(anonymous namespace)::BenchmarkResultsPrinter*, folly::(anonymous namespace)::BenchmarksToRun const&)
# 8  std::pair<std::set<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::vector<folly::detail::BenchmarkResult, std::allocator<folly::detail::BenchmarkResult> > > folly::detail::BenchmarkingStateBase::runBenchmarksWithPrinter<folly::(anonymous namespace)::BenchmarkResultsPrinter>(folly::(anonymous namespace)::BenchmarkResultsPrinter*) const
# 9  folly::runBenchmarks()
# 10 main
# 11 0x0000000000029d8f
# 12 __libc_start_main
# 13 _start

*** Aborted at 1740059898 (Unix time, try 'date -d 1740059898') ***
*** Signal 6 (SIGABRT) (0x3e800116923) received by PID 1141027 (pthread TID 0x7f58c75a4440) (linux TID 1141027) (maybe from PID 1141027, UID 1000) (code: -6), stack trace: ***
    @ 0000000003a84e11 folly::symbolizer::(anonymous namespace)::signalHandler(int, siginfo_t*, void*)
                       /var/git/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/folly/folly/experimental/symbolizer/SignalHandler.cpp:453
    @ 000000000004251f (unknown)
    @ 00000000000969fc pthread_kill
    @ 0000000000042475 raise
    @ 00000000000287f2 abort
    @ 00000000000a2b9d (unknown)
    @ 00000000000ae20b (unknown)
    @ 00000000000ae276 std::terminate()
    @ 00000000000ae4d7 __cxa_throw
    @ 00000000039a457a __cxa_throw
                       /var/git/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/folly/folly/debugging/exception_tracer/ExceptionTracerLib.cpp:159
    @ 00000000072ec6a1 void facebook::velox::detail::veloxCheckFail<facebook::velox::VeloxRuntimeError, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(facebook::velox::detail::VeloxCheckFailArgs const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
    @ 00000000058de625 facebook::velox::exec::Task::checkExecutionMode(facebook::velox::exec::Task::ExecutionMode)
    @ 00000000058f8ac3 facebook::velox::exec::Task::next(folly::SemiFuture<folly::Unit>*)
    @ 000000000286b9cc (anonymous namespace)::SimpleAggregatesBenchmark::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
    @ 0000000002878923 std::_Function_handler<folly::detail::TimeIterData (unsigned int), folly::detail::BenchmarkingState<std::chrono::_V2::system_clock>::addBenchmark<(anonymous namespace)::follyBenchmarkUnused16::{lambda(unsigned int)https://github.com/facebookincubator/velox/issues/1}&>(char const*, folly::Range<char const>, std::enable_if&&)::{lambda(unsigned int)https://github.com/facebookincubator/velox/issues/1}>::_M_invoke(std::_Any_data const&, unsigned int&&)
    @ 0000000003a9e426 folly::runBenchmarkGetNSPerIteration(std::function<folly::detail::TimeIterData (unsigned int)> const&, double)
                       /usr/include/c++/11/bits/std_function.h:590
                       -> /var/git/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/folly/folly/Benchmark.cpp
    @ 0000000003aa5571 folly::(anonymous namespace)::runBenchmarksWithPrinterImpl(folly::(anonymous namespace)::BenchmarkResultsPrinter*, folly::(anonymous namespace)::BenchmarksToRun const&)
                       /var/git/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/folly/folly/Benchmark.cpp:773
    @ 0000000003aa7108 std::pair<std::set<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::vector<folly::detail::BenchmarkResult, std::allocator<folly::detail::BenchmarkResult> > > folly::detail::BenchmarkingStateBase::runBenchmarksWithPrinter<folly::(anonymous namespace)::BenchmarkResultsPrinter>(folly::(anonymous namespace)::BenchmarkResultsPrinter*) const
                       /var/git/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/folly/folly/Benchmark.cpp:880
    @ 0000000003aa80ef folly::runBenchmarks()
                       /var/git/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/folly/folly/Benchmark.cpp:920
    @ 000000000278bd04 main
    @ 0000000000029d8f (unknown)
    @ 0000000000029e3f __libc_start_main
    @ 00000000027b1144 _start
```

Pull Request resolved: #12400

Reviewed By: xiaoxmeng

Differential Revision: D69969638

Pulled By: mbasmanova

fbshipit-source-id: 8a861afea94ca9fda8e0d8e842f5f13d1647f7b6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kagamiori kagamiori Awaiting requested review from kagamiori

@assignUser assignUser Awaiting requested review from assignUser assignUser is a code owner

@majetideepak majetideepak Awaiting requested review from majetideepak majetideepak is a code owner

@mbasmanova mbasmanova Awaiting requested review from mbasmanova

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zhli1142015 @mbasmanova @facebook-github-bot