From a2d1738150a6bc2bda43eb903fb1d7d859528f47 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 18 Jul 2022 19:02:22 +0000 Subject: [PATCH 1/7] Add a sentence on writing perf tests for SQL functions --- docs/en/development/tests.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/en/development/tests.md b/docs/en/development/tests.md index 8ce38178020..4d85934f730 100644 --- a/docs/en/development/tests.md +++ b/docs/en/development/tests.md @@ -81,11 +81,11 @@ $ ./src/unit_tests_dbms --gtest_filter=LocalAddress* ## Performance Tests {#performance-tests} -Performance tests allow to measure and compare performance of some isolated part of ClickHouse on synthetic queries. Tests are located at `tests/performance`. Each test is represented by `.xml` file with description of test case. Tests are run with `docker/test/performance-comparison` tool . See the readme file for invocation. +Performance tests allow to measure and compare performance of some isolated part of ClickHouse on synthetic queries. Performance tests are located at `tests/performance/`. Each test is represented by an `.xml` file with a description of the test case. Tests are run with `docker/test/performance-comparison` tool . See the readme file for invocation. Each test run one or multiple queries (possibly with combinations of parameters) in a loop. -If you want to improve performance of ClickHouse in some scenario, and if improvements can be observed on simple queries, it is highly recommended to write a performance test. It always makes sense to use `perf top` or other `perf` tools during your tests. +If you want to improve performance of ClickHouse in some scenario, and if improvements can be observed on simple queries, it is highly recommended to write a performance test. Also, it is recommended to write performance tests when you add or modify SQL functions which are relatively isolated and not too obscure. It always makes sense to use `perf top` or other `perf` tools during your tests. ## Test Tools and Scripts {#test-tools-and-scripts} From b09121df11eb9a15597f0b4f239a2245ad09408c Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 18 Jul 2022 19:05:32 +0000 Subject: [PATCH 2/7] Remove comment to check for CMake version a) the build script aborts anyways if CMake is too old b) our minimally required version is >3 years old, so the chance of an abort due to outdated CMake is fairly low c) removing the hint to check the version removes the need to constantly update the version in the docs --- docs/en/development/developer-instruction.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/en/development/developer-instruction.md b/docs/en/development/developer-instruction.md index 77ddae6a756..dc3bb07f7c1 100644 --- a/docs/en/development/developer-instruction.md +++ b/docs/en/development/developer-instruction.md @@ -124,8 +124,6 @@ For installing CMake and Ninja on Mac OS X first install Homebrew and then insta /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)" brew install cmake ninja -Next, check the version of CMake: `cmake --version`. If it is below 3.12, you should install a newer version from the website: https://cmake.org/download/. - ## C++ Compiler {#c-compiler} Compilers Clang starting from version 11 is supported for building ClickHouse. From cec8458429b2192e7ec8910b9435526b7480006a Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 18 Jul 2022 19:05:53 +0000 Subject: [PATCH 3/7] Remove Mac instructions ... they are given at length on the dedicated setup page for Mac --- docs/en/development/developer-instruction.md | 8 -------- 1 file changed, 8 deletions(-) diff --git a/docs/en/development/developer-instruction.md b/docs/en/development/developer-instruction.md index dc3bb07f7c1..53443fd12bb 100644 --- a/docs/en/development/developer-instruction.md +++ b/docs/en/development/developer-instruction.md @@ -119,11 +119,6 @@ On CentOS, RedHat run `sudo yum install cmake ninja-build`. If you use Arch or Gentoo, you probably know it yourself how to install CMake. -For installing CMake and Ninja on Mac OS X first install Homebrew and then install everything else via brew: - - /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)" - brew install cmake ninja - ## C++ Compiler {#c-compiler} Compilers Clang starting from version 11 is supported for building ClickHouse. @@ -136,9 +131,6 @@ On Ubuntu/Debian you can use the automatic installation script (check [official sudo bash -c "$(wget -O - https://apt.llvm.org/llvm.sh)" ``` -Mac OS X build is also supported. Just run `brew install llvm` - - ## The Building Process {#the-building-process} Now that you are ready to build ClickHouse we recommend you to create a separate directory `build` inside `ClickHouse` that will contain all of the build artefacts: From 2ded3da8876ce497c365e2eb21abd8d2fa5797e4 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 18 Jul 2022 19:06:50 +0000 Subject: [PATCH 4/7] Bump Clang version to current minimum --- docs/en/development/developer-instruction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/development/developer-instruction.md b/docs/en/development/developer-instruction.md index 53443fd12bb..ea2ed95fd27 100644 --- a/docs/en/development/developer-instruction.md +++ b/docs/en/development/developer-instruction.md @@ -121,7 +121,7 @@ If you use Arch or Gentoo, you probably know it yourself how to install CMake. ## C++ Compiler {#c-compiler} -Compilers Clang starting from version 11 is supported for building ClickHouse. +Compilers Clang starting from version 12 is supported for building ClickHouse. Clang should be used instead of gcc. Though, our continuous integration (CI) platform runs checks for about a dozen of build combinations. From a9fb677084884ba345ae81aa2832d5306ec08e80 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 18 Jul 2022 19:12:02 +0000 Subject: [PATCH 5/7] Bump Clang version to current minimum --- docs/en/development/style.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/development/style.md b/docs/en/development/style.md index 0c99426db03..25c51527b88 100644 --- a/docs/en/development/style.md +++ b/docs/en/development/style.md @@ -745,7 +745,7 @@ But other things being equal, cross-platform or portable code is preferred. **2.** Language: C++20 (see the list of available [C++20 features](https://en.cppreference.com/w/cpp/compiler_support#C.2B.2B20_features)). -**3.** Compiler: `clang`. At this time (April 2021), the code is compiled using clang version 11. (It can also be compiled using `gcc` version 10, but it's untested and not suitable for production usage). +**3.** Compiler: `clang`. At the time of writing (July 2022), the code is compiled using clang version >= 12. (It can also be compiled using `gcc`, but it's untested and not suitable for production usage). The standard library is used (`libc++`). From f3a60991f4135c516ba1036f9e9f843c62e41592 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 18 Jul 2022 19:12:33 +0000 Subject: [PATCH 6/7] Smallish compiler warnings update --- docs/en/development/style.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/development/style.md b/docs/en/development/style.md index 25c51527b88..e949aba9ea2 100644 --- a/docs/en/development/style.md +++ b/docs/en/development/style.md @@ -755,7 +755,7 @@ The standard library is used (`libc++`). The CPU instruction set is the minimum supported set among our servers. Currently, it is SSE 4.2. -**6.** Use `-Wall -Wextra -Werror` compilation flags. Also `-Weverything` is used with few exceptions. +**6.** Use `-Wall -Wextra -Werror -Weverything` compilation flags with a few exception. **7.** Use static linking with all libraries except those that are difficult to connect to statically (see the output of the `ldd` command). From 68f0dcc20624a25cef903b202e304a3403b4f7e3 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 18 Jul 2022 19:13:00 +0000 Subject: [PATCH 7/7] Remove mention of C++03 exception specifiers ... such exception specifiers trigger a compiler warning these days anyways (i.e. are treated as error) --- docs/en/development/style.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/en/development/style.md b/docs/en/development/style.md index e949aba9ea2..a543c7532f8 100644 --- a/docs/en/development/style.md +++ b/docs/en/development/style.md @@ -692,9 +692,7 @@ auto s = std::string{"Hello"}; **1.** Virtual inheritance is not used. -**2.** Exception specifiers from C++03 are not used. - -**3.** Constructs which have convenient syntactic sugar in modern C++, e.g. +**2.** Constructs which have convenient syntactic sugar in modern C++, e.g. ``` // Traditional way without syntactic sugar