-
Notifications
You must be signed in to change notification settings - Fork 618
[VL] Use Velox TaskCursor infrastructure #12302
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
Changes from 6 commits
8fd476e
fa02939
f44d674
ee7dd59
aff4d87
1c1200c
312f45b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ | |
| #include "shuffle/ShuffleReader.h" | ||
| #include "shuffle/ShuffleWriter.h" | ||
| #include "substrait/plan.pb.h" | ||
| #include "threads/ThreadManager.h" | ||
| #include "utils/ObjectStore.h" | ||
| #include "utils/WholeStageDumper.h" | ||
|
|
||
|
|
@@ -66,12 +67,14 @@ class Runtime : public std::enable_shared_from_this<Runtime> { | |
| using Factory = std::function<Runtime*( | ||
| const std::string& kind, | ||
| MemoryManager* memoryManager, | ||
| ThreadManager* threadManager, | ||
| const std::unordered_map<std::string, std::string>& sessionConf)>; | ||
| using Releaser = std::function<void(Runtime*)>; | ||
| static void registerFactory(const std::string& kind, Factory factory, Releaser releaser); | ||
| static Runtime* create( | ||
| const std::string& kind, | ||
| MemoryManager* memoryManager, | ||
| ThreadManager* threadManager, | ||
| const std::unordered_map<std::string, std::string>& sessionConf = {}); | ||
| static void release(Runtime*); | ||
| static std::optional<std::string>* localWriteFilesTempPath(); | ||
|
|
@@ -80,8 +83,9 @@ class Runtime : public std::enable_shared_from_this<Runtime> { | |
| Runtime( | ||
| const std::string& kind, | ||
| MemoryManager* memoryManager, | ||
| ThreadManager* threadManager, | ||
| const std::unordered_map<std::string, std::string>& confMap) | ||
| : kind_(kind), memoryManager_(memoryManager), confMap_(confMap) {} | ||
| : kind_(kind), memoryManager_(memoryManager), threadManager_(threadManager), confMap_(confMap) {} | ||
|
|
||
| virtual ~Runtime() = default; | ||
|
|
||
|
|
@@ -131,6 +135,10 @@ class Runtime : public std::enable_shared_from_this<Runtime> { | |
| return memoryManager_; | ||
| }; | ||
|
|
||
| virtual ThreadManager* threadManager() { | ||
| return threadManager_; | ||
| }; | ||
|
|
||
| /// This function is used to create certain converter from the format used by | ||
| /// the backend to Spark unsafe row. | ||
| virtual std::shared_ptr<ColumnarToRowConverter> createColumnar2RowConverter(int64_t column2RowMemThreshold) { | ||
|
|
@@ -189,6 +197,7 @@ class Runtime : public std::enable_shared_from_this<Runtime> { | |
| protected: | ||
| std::string kind_; | ||
| MemoryManager* memoryManager_; | ||
| ThreadManager* threadManager_; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. was trying to understand the lifetime for this
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. The destruction is guaranteed to be right earlier than destruction of memory manager, by its priority settings:
I was aware of this, and this kind of leakage was fundamentally fixed at once by #11882, specifically, by the introduction of HookedExecutor. HookedExecutor can reuse OS thread resources, while it guarantees no background threads are running before we end the Velox task. Precondition is, Velox should keep avoiding running threads off the thread pools we pass to it, and this rule is unlikely to be broken by Velox shortly. |
||
| std::unique_ptr<ObjectStore> objStore_ = ObjectStore::create(); | ||
| std::unordered_map<std::string, std::string> confMap_; // Session conf map | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| #include "ThreadInitializer.h" | ||
|
|
||
| namespace gluten { | ||
| namespace { | ||
|
|
||
| /// A ThreadInitializer whose initialize() and destroy() are no-ops. | ||
| /// Used when no JVM or Spark task context is available (e.g., benchmarks). | ||
| class NoopThreadInitializer final : public ThreadInitializer { | ||
| public: | ||
| void initialize(const std::string& threadName) override {} | ||
| void destroy(const std::string& threadName) override{}; | ||
| }; | ||
|
|
||
| } // namespace | ||
|
|
||
| std::unique_ptr<ThreadInitializer> ThreadInitializer::noop() { | ||
| return std::make_unique<NoopThreadInitializer>(); | ||
| } | ||
|
|
||
| } // namespace gluten |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems this value is set to -1 on scala side
gluten/gluten-core/src/main/scala/org/apache/gluten/config/GlutenCoreConfig.scala
Line 173 in 3b08a23
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's finally set here:
gluten/gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala
Line 145 in 3b08a23