Feat[WIP]: JIT compiled SQL expressions #2891

jun0315 · 2023-07-04T08:50:53Z

Summary

MCS uses interepreted execution to calculate SQL expressions results. Here is an example of a SQL expression 'table1.col1 + FLOOR(table2.col1)'. Given that table1.col1 is DECIMAL and table2.col1 is DOUBLE there is a number of conditions that drives the calculation of this relatively simple example in runtime. Given that SQL types and expression tree are known before the query begins it is possible to replace interpretation with JIT to produce specialized compiled bytecode that is:

small
has no or almost no branches
optimized for the specific platform it is run at
This is a research mostly project which goal is to produce a set of microbenchmarks that:

leverages any JIT compiler available, e.g. LLVM, MIR
demonstrates a negative and positive effects of using JIT

I tried to compile this in the submodule storage/columnstore/columnstore under MariaDB/server@313c5a1. I got the build to complete, but the directory storage/columnstore/columnstore/jit/CMakeFiles/jit.dir in my cmake -G Ninja based build directory is empty. Other ColumnStore compilation units were being built.

dr-m · 2023-07-06T11:10:33Z

jit/jit.cpp

+  module_identifier_to_memory_manager.erase(module_memory_manager_it);
+  compiled_code_size.fetch_sub(module.size, std::memory_order_relaxed);


Which code or data structure owns the memory that module referes to here? Could the memory actually be freed in the erase call? If so, then these statements should be swapped with each other to make the code safe.

dr-m · 2023-07-06T11:13:49Z

jit/jit.h

+  // TODO lock when compiling a module
+  mutable std::mutex jit_lock;


For any mutex or any potentially concurrently accessed data structure, it would be useful to document the protection rules. Many of these data members are actually protected by jit_lock in deleteCompiledModule().

dr-m · 2023-07-06T11:19:44Z

jit/jit.h

+  std::unique_ptr<JITSymbolResolver> symbol_resolver;
+  std::unordered_map<uint64_t, std::unique_ptr<JITModuleMemoryManager>> module_identifier_to_memory_manager;
+  uint64_t current_module_key = 0;
+  std::atomic<size_t> compiled_code_size;


I’d like to see comment for documenting the lifetime and purpose of each data member. For compiled_code_size it would be useful to know what (if any) kind of padding or alignment overhead it would include.

Due to the lack of comments, it took me some time to find a counterpart of deleteCompiledModule() that would increment compiled_code_size and add something to module_identifier_to_memory_manager. It is compileModule(std::unique_ptr<llvm::Module>). It seems to lack proper jit_lock protection.

dr-m · 2023-07-06T11:20:55Z

jit/jit.cpp

+  Arena() : page_size(8)
+  {
+  }


We could live with an implicit default constructor if the initializer expression for page_size was revised to this value.

dr-m · 2023-07-06T11:26:19Z

jit/jit.cpp

+  inline size_t getPageSize() const
+  {
+    return page_size;
+  }
+
+ private:
+  size_t page_size = 0;


I do not see any modification of this page_size in the code. I think that it could be declared simply as

const size_t page_size = 8;

or even static const or static constexpr to save some memory, and then we could avoid explicitly declaring Arena(). I think that it is rather pointless to declare const data members private and to define public accessors for them.

What is the unit here? Bytes? Kilobytes? Some comments would be helpful.

Typically, sysconf(PAGESIZE) should return 4096 or a larger value.

dr-m · 2023-07-06T11:29:03Z

utils/funcexp/functor.h

+  bool isCompilable(const execplan::CalpontSystemCatalog::ColType& colType)
+  {
+    return false;
+  }
+  llvm::Value* compile()
+  {
+  }


It looks like the actual interesting part is still missing. The body of compile() seems to be missing return nullptr;.

dr-m · 2023-07-06T11:30:09Z

jit/compilefunction.cpp

+void compileFunction(llvm::Module& module, const Func& function)
+{
+  llvm::IRBuilder<> b(module.getContext());
+  function->
+}


The member function body does not look like valid C++ to me.

dr-m · 2023-07-06T11:35:02Z

utils/funcexp/functor.h

@@ -38,6 +38,8 @@

 #include "dataconvert.h"

+#include <llvm/IR/IRBuilder.h>


This is a rather large header file that will #include many other header files. I think that including that kind of a "jungle" in a rather popular header file (functor.h is being included from many other header files, which in turn could be included from many compilation units) could significantly include the compilation time.

We only seem to need this for the type name llvm::Value, which compile() would return a pointer to. I think that it would be better to define an opaque type and avoid including the header here. Something like this might work:

namespace llvm { class Value; }

dr-m · 2023-08-24T11:26:19Z

jit/compileoperator.cpp

+  }
+  auto* func_type = llvm::FunctionType::get(return_type, {data_type, isNull_type}, false);
+  auto* func =
+      llvm::Function::Create(func_type, llvm::Function::ExternalLinkage, expression->alias(), module);


I would suggest

func->setDoesNotThrow();

so that no .eh_frame or .rela.eh_frame section will be generated.

dr-m · 2023-08-24T11:30:16Z

jit/compileoperator.h

+  JITCompiledOperator<int64_t> compiled_function_int64;
+  JITCompiledOperator<uint64_t> compiled_function_uint64;
+  JITCompiledOperator<double> compiled_function_double;
+  JITCompiledOperator<float> compiled_function_float;


Can multiple compiled_function_ be set at a time?

No, only one pointer will be set at a time.

dr-m · 2023-08-24T11:32:14Z

jit/expressionjit.h

+  int64_t getIntVal(rowgroup::Row& row, bool& isNull) override
+  {
+    return compiledOperator.compiled_function_int64(row.getData(), isNull);
+  }
+  uint64_t getUintVal(rowgroup::Row& row, bool& isNull) override
+  {
+    return compiledOperator.compiled_function_int64(row.getData(), isNull);
+  }


compiled_function_uint64 seems to be unused. Do we really need this distinction? The ABI for signed and unsigned should be the same.

I would suggest to add some tests and to demonstrate that all this code is covered by the test cases.

dr-m · 2023-08-24T11:33:19Z

jit/expressionjit.h

+  double getDoubleVal(rowgroup::Row& row, bool& isNull) override
+  {
+    return compiledOperator.compiled_function_double(row.getData(), isNull);
+  }
+  float getFloatVal(rowgroup::Row& row, bool& isNull) override
+  {
+    return compiledOperator.compiled_function_float(row.getData(), isNull);
+  }


Could we simply have one compiled_function member that will be reinterpret_cast to the appropriate type? That could avoid some run-time code duplication.

drrtuy · 2023-09-18T12:22:43Z

CMakeLists.txt

@@ -29,7 +29,7 @@ endif()
 INCLUDE(ExternalProject)
 INCLUDE(CheckCXXSourceCompiles)

-SET(CMAKE_CXX_STANDARD 11)
+SET(CMAKE_CXX_STANDARD 14)


We use c++20 if compiler handles it via compiler flags. Is this set is necessary?

It is necessary. If it is not specified as 14 during compilation, an error will be reported.

drrtuy · 2023-09-18T12:23:34Z

CMakeLists.txt

@@ -358,6 +372,7 @@ ADD_SUBDIRECTORY(writeengine/splitter)
 ADD_SUBDIRECTORY(storage-manager)
 ADD_SUBDIRECTORY(datatypes)
 ADD_SUBDIRECTORY(tests)
+ADD_SUBDIRECTORY(jit)


This directory must be moved into ./utils and the paths changed accordingly.

drrtuy · 2023-09-18T12:29:21Z

jit/CMakeLists.txt

+
+add_library(jit SHARED ${jit_LIB_SRCS})
+
+target_link_libraries(jit ${NETSNMP_LIBRARIES} ${LLVM_LIBS} ${LLVM_SYSTEM_LIBS} ${LLVM_LDFLAGS})


You can drop ${NETSNMP_LIBRARIES} as this is an obsolete prerequisite.

drrtuy · 2023-09-18T12:51:16Z

jit/compileoperator.cpp

+{
+  auto columns = expression.get()->simpleColumnList();
+  llvm::IRBuilder<> b(module.getContext());
+  compileExternalFunction(module, b);


How many times compileExternalFunction will be actually called?

drrtuy · 2023-09-18T12:53:28Z

jit/compileoperator.cpp

+  b.CreateRet(ret);
+
+  // Print the IR
+  llvm::verifyFunction(*func);


I suppose these two lines will go away.

drrtuy · 2023-09-18T13:13:45Z

dbcon/execplan/arithmeticoperator.h

+  inline llvm::Value* compile(llvm::IRBuilder<>& b, llvm::Value* data, llvm::Value* isNull,
+                              rowgroup::Row& row, ParseTree* lop, ParseTree* rop,
+                              CalpontSystemCatalog::ColDataType dataType) override;
+  inline llvm::Value* compileI(llvm::IRBuilder<>& b, llvm::Value* l, llvm::Value* r);


Suggested change

inline llvm::Value* compileI(llvm::IRBuilder<>& b, llvm::Value* l, llvm::Value* r);

inline llvm::Value* compileInt_(llvm::IRBuilder<>& b, llvm::Value* l, llvm::Value* r);

drrtuy · 2023-09-18T13:14:00Z

dbcon/execplan/arithmeticoperator.h

+                              rowgroup::Row& row, ParseTree* lop, ParseTree* rop,
+                              CalpontSystemCatalog::ColDataType dataType) override;
+  inline llvm::Value* compileI(llvm::IRBuilder<>& b, llvm::Value* l, llvm::Value* r);
+  inline llvm::Value* compileF(llvm::IRBuilder<>& b, llvm::Value* l, llvm::Value* r);


Suggested change

inline llvm::Value* compileF(llvm::IRBuilder<>& b, llvm::Value* l, llvm::Value* r);

inline llvm::Value* compileFloat_(llvm::IRBuilder<>& b, llvm::Value* l, llvm::Value* r);

utils/rowgroup/rowgroup.cpp

[mod] add CMake config for llvm. [feat] function [feat] msc_jit [fix] errors caused by namespace [mod] resolved the build error

[feat] adjust the structure of the compile method and implement IR for expression [feat] add some testing code for JIT [fix] arena memory allocation error issue [mod] rollback the lib of llvm to the system built-in library [feat] add JIT optimization for simpleColumnInt type [feat] LogicOperator compile

[mod] add isCompilable for column [mod] add check for null row [fix] the issue of compilation errors in the algorithm column IR

[mod] remove some useless param [mod] remove some useless file [feat] add support for day() function [feat] jit add support for func_month() function

[feat] func_day add support for timestamp [feat] jit add external function timestampValueToInt [mod] Modify the implementation of compileExpression

[fix] the logic for func_hour [feat] add the jit support for func_second [mod] delete some useless define

[mod] move jit to utils dir [mod] remove link for NETSNMP_LIBRARIES [mod] standardize the function name

@ubuntu20

add temp add temp [fix] some Werror feat(cmake): hardcoded llvm include paths for a system llvm-13-dev @ubuntu20 WIP rebase on develop and change the code to compile with system llvm libs >= llvm-13

no hardcoded llvm version

contos7 llvm cmake config llvm conditional include

add llvm deps centos7 dep fix

update llvm to 12 on Ubuntu 20 and to 16 on Debian11 WIP llvm-16 compilation issue

WIP WIP compilation fix WIP introduce an expression name to disambigue JIT symbols in a JIT module

WIP compilation fix

WIP avoid SECOND(TIMESTAMP) compilation

mariadb-LeonidFedorov · 2024-07-31T13:51:53Z

work moved to #3165

drrtuy and others added 6 commits March 10, 2023 08:31

Merge pull request mariadb-corporation#2766 from mariadb-corporation/…

58da5ee

…build-dev6 Add ability to compile against MariaDB with new cost model

[add] jit init

76e7ada

[feat] compilation function of JIT module

0dd7229

[feat] add implement of JITModuleMemoryManager

e9ec71b

[feat] add implement of deleteCompiledModule and registerExternalSymbol

35ab7de

[feat] add implement of arena memory pool

bbef873

drrtuy requested review from drrtuy and dr-m July 5, 2023 10:07

dr-m reviewed Jul 6, 2023

View reviewed changes

dr-m reviewed Aug 24, 2023

View reviewed changes

drrtuy reviewed Sep 18, 2023

View reviewed changes

utils/rowgroup/rowgroup.cpp Outdated Show resolved Hide resolved

drrtuy marked this pull request as ready for review September 25, 2023 19:21

jun0315 marked this pull request as draft June 2, 2024 17:01

jun0315 added 9 commits June 3, 2024 22:21

[feat] implement optimization functions for modules

36f22c1

[mod] add CMake config for llvm. [feat] function [feat] msc_jit [fix] errors caused by namespace [mod] resolved the build error

[feat] PredicateOperator compile

9f53a70

[mod] add isCompilable for column [mod] add check for null row [fix] the issue of compilation errors in the algorithm column IR

[fix] jit add support for double and float type

2fa3f0d

[mod] remove some useless param [mod] remove some useless file [feat] add support for day() function [feat] jit add support for func_month() function

[feat] func_day add support for call external function

923c203

[feat] func_day add support for timestamp [feat] jit add external function timestampValueToInt [mod] Modify the implementation of compileExpression

[feat] jit supports more types of date conversions

25988e3

[fix] the logic for func_hour [feat] add the jit support for func_second [mod] delete some useless define

[fix] data type transform error

2660910

[mod] move jit to utils dir [mod] remove link for NETSNMP_LIBRARIES [mod] standardize the function name

[mod] move type compile function to compilehelper

7882658

add temp add temp [fix] some Werror feat(cmake): hardcoded llvm include paths for a system llvm-13-dev @ubuntu20 WIP rebase on develop and change the code to compile with system llvm libs >= llvm-13

llvm rpms for ci

8194af5

no hardcoded llvm version

jun0315 added 6 commits June 3, 2024 22:23

llvm for deb

fd18740

contos7 llvm cmake config llvm conditional include

__STDC_LIMIT_MACROS fix

c1b78b5

add llvm deps centos7 dep fix

add libjit to deb packages

bbf1f8d

update llvm to 12 on Ubuntu 20 and to 16 on Debian11 WIP llvm-16 compilation issue

WIP RowGroup helper class

32f1184

WIP WIP compilation fix WIP introduce an expression name to disambigue JIT symbols in a JIT module

WIP fixed cpimport rollback related issues in tests

b9c8b5e

WIP compilation fix

WIP fix for SECOND() func

e71b626

WIP avoid SECOND(TIMESTAMP) compilation

jun0315 force-pushed the jit-dev-11.1 branch from 86eac7a to e71b626 Compare June 3, 2024 14:56

mariadb-LeonidFedorov closed this Jul 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat[WIP]: JIT compiled SQL expressions #2891

Feat[WIP]: JIT compiled SQL expressions #2891

jun0315 commented Jul 4, 2023 •

edited

Loading

dr-m left a comment

dr-m Jul 6, 2023

dr-m Jul 6, 2023

dr-m Jul 6, 2023

dr-m Jul 6, 2023

dr-m Jul 6, 2023

dr-m Jul 6, 2023

dr-m Jul 6, 2023

dr-m Jul 6, 2023

dr-m Aug 24, 2023

noorall Sep 22, 2023

dr-m Aug 24, 2023

noorall Sep 18, 2023

dr-m Aug 24, 2023

dr-m Aug 24, 2023

drrtuy Sep 18, 2023

noorall Sep 22, 2023

drrtuy Sep 18, 2023

noorall Sep 22, 2023

drrtuy Sep 18, 2023

noorall Sep 22, 2023

drrtuy Sep 18, 2023

drrtuy Sep 18, 2023

noorall Sep 22, 2023

drrtuy Sep 18, 2023

drrtuy Sep 18, 2023

mariadb-LeonidFedorov commented Jul 31, 2024

		module_identifier_to_memory_manager.erase(module_memory_manager_it);
		compiled_code_size.fetch_sub(module.size, std::memory_order_relaxed);

		// TODO lock when compiling a module
		mutable std::mutex jit_lock;

		@@ -38,6 +38,8 @@

		#include "dataconvert.h"

		#include <llvm/IR/IRBuilder.h>


		add_library(jit SHARED ${jit_LIB_SRCS})

		target_link_libraries(jit ${NETSNMP_LIBRARIES} ${LLVM_LIBS} ${LLVM_SYSTEM_LIBS} ${LLVM_LDFLAGS})

	inline llvm::Value* compileI(llvm::IRBuilder<>& b, llvm::Value* l, llvm::Value* r);
	inline llvm::Value* compileInt_(llvm::IRBuilder<>& b, llvm::Value* l, llvm::Value* r);

	inline llvm::Value* compileF(llvm::IRBuilder<>& b, llvm::Value* l, llvm::Value* r);
	inline llvm::Value* compileFloat_(llvm::IRBuilder<>& b, llvm::Value* l, llvm::Value* r);

Feat[WIP]: JIT compiled SQL expressions #2891

Feat[WIP]: JIT compiled SQL expressions #2891

Conversation

jun0315 commented Jul 4, 2023 • edited Loading

Summary

Related

dr-m left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mariadb-LeonidFedorov commented Jul 31, 2024

jun0315 commented Jul 4, 2023 •

edited

Loading