Support limit, offset, top-k serial and parallel versions in codegen #1435

pmenon · 2018-06-27T12:24:21Z

After #1385 was merged in, I resurrected LIMIT, OFFSET from my old branch, and implemented Top-K optimizations. These both work for serial and parallel execution, and are needed for TPC-C.

Not quite ready for merging in, but folks can take a look at it.

tcm-marcel

The code looks good, but I haven't tried it out yet. But I do have some questions: (this one and some in the code)

It seems like limit and offset are compile-time constants. That means, the compiled plan cannot be reused for other limit/offset values. Is that correct?

tcm-marcel · 2018-06-29T10:58:22Z

src/codegen/aggregation.cpp

@@ -417,7 +417,7 @@ void Aggregation::DoNullCheck(
        default: { break; }
      }
    }
-    agg_is_null.ElseBlock("Agg.IfAggIsNotNull");


Is there a reason to remove the naming? I found all these names very helpful when looking at IR. I know that all the string handling introduces some overhead, maybe we could drop then in Release mode.

Most students will probably use printfs to trace through the code. We still allow naming of the 'if' statements and the IR tracks predecessors which conveys the same information.

tcm-marcel · 2018-06-29T11:16:30Z

src/include/codegen/interpreter/bytecode_instructions.def

 HANDLE_EXPLICIT_CALL_INST(peloton_sorter_sort,
                          peloton::codegen::util::Sorter::Sort)
 HANDLE_EXPLICIT_CALL_INST(peloton_sorter_sortparallel,
                          peloton::codegen::util::Sorter::SortParallel)
+HANDLE_EXPLICIT_CALL_INST(peloton_sorter_sorttopkparallel,
+                          peloton::codegen::util::Sorter::SortTopKParallel)


I see you added more explicit call instructions for the bytecode interpreter. FYI: Once the interpreter is connected, it will print a debug message for every function call where no explicit call instruction exists, so it will be easy to find those.

tcm-marcel · 2018-06-29T12:07:52Z

src/codegen/function_builder.cpp

+  }
+
+  // Generate loading code and save in cache
+  val = load_func();


Where does the caching happen?

The loaded values are inserted into the cached_vars_ member variable that is probed earlier.

tcm-marcel · 2018-06-29T12:23:34Z

src/codegen/operator/limit_translator.cpp

+    codegen->CreateStore(next_limit, limit_count_ptr);
+  } else {
+    // Parallel mode. Atomically increment the count.
+    next_limit = codegen->CreateAtomicRMW(


Do you know what this looks like in IR and what LLVM compiles it to? (I haven't look at it yet, Google was not helpful) Whatever it is - I have the feeling that the Interpreter doesn't support it yet.

We generate LLVM's atomicrmw with sequential consistency. This may lower to different instructions depending on architecture, but on the few Intel Skylake+ machines I tested, it uses lock xadd.

In the interpreter, you can use our peloton::atomic_add(), which boils down to a locked xadd.

tcm-marcel · 2018-06-29T12:33:56Z

src/codegen/operator/order_by_translator.cpp

-      child_pipeline_(this, Pipeline::Parallelism::Serial) {
-  // Aggregations happen serially (for now ...)
-  pipeline.SetSerial();
+      child_pipeline_(this, Pipeline::Parallelism::Flexible) {


Do I understand correctly: The child pipeline of the Orderby (before the hashing) can be parallel, but the order-by itself has to be serial because of the aggregations.

This is saying is that the consumer portion of the order-by (i.e., the child pipeline) can be executed in parallel, and that the production portion (i.e., the scanning of the sorter) will be serial. It has no information about other operators in the pipeline.

tcm-marcel · 2018-06-29T12:42:28Z

src/codegen/util/sorter.cpp

@@ -97,8 +131,11 @@ void Sorter::Sort() {

  timer.Stop();

-  LOG_DEBUG("Sorted %zu tuples in %.2f ms", tuples_.size(),
-            timer.GetDuration());
+#ifndef NDEBUG


This should be #ifdef LOG_DEBUG_ENABLED

tcm-marcel · 2018-06-29T12:44:14Z

src/include/codegen/util/sorter.h

+   * Sift down the element at the root of the heap while maintaining the heap
+   * property.
+   */
+  void HeapSiftDown();


What is the difference of this function and STL push_heap()?

In essence, they are the same. But, push_heap alone doesn't give us the functionality we need. We need to remove the smallest (largest) element off the min (max) heap and insert a new element. We could use a pair of push_heap and pop_heap calls, but we've managed to achieve the same functionality with a single O(logn) operation.

coveralls · 2018-07-01T03:57:26Z

Coverage increased (+0.05%) to 76.421% when pulling 0e2de1e on pmenon:limit into 2406b76 on cmu-db:master.

…k exit

…o bytecode builder. Fixed parallel sorting top-k

…s were equal producing non-deterministic outputs). Fixed order by translator test to push a limit plan on top. This wasn't there before because we didn't need a limit

pmenon · 2018-07-22T16:21:12Z

@tcm-marcel Want to take a last crack at this? Don't want this to get stale.

tcm-marcel

Sorry this took so long. I think it is good to go!

apavlo · 2018-08-03T12:10:08Z

@tcm-marcel I think this is stuck on the Jenkins mac build?

tcm-marcel · 2018-08-03T15:20:41Z

The Jenkins Mac build fails because no space is left on the hard drive:

/tmp/workspace/peloton_PR-1435-XOL5JG3LYATJV2HWKIVAIOIZQP2OCJC5FY5VVGBXQZ6BT33JVMUA@2: No space left on device

@apavlo I am not familiar with the Jenkins Mac machine. But on Travis the Mac build succeeds.

apavlo · 2018-08-04T02:34:12Z

@crd477 Can you check this?

crd477 · 2018-08-06T14:01:06Z

Fixed with the quick fix of restarting the VM from a clean snapshot. The more complete fix of how to do this cleanly and regularly was still on my TODO list before I left for vacation last week and I'll get back to it today.
I also restarted the Jenkins builds for master and #1435. They should pass soon.

crd477 · 2018-08-07T19:57:12Z

The macOS Jenkins worker should be stable now. I grew the VM's disk and added a workspace cleanup cronjob as the other Jenkins workers have.

…mu-db#1435) * Initial limit commit * Cleanup OrderByPlan * Don't use SQL types for the result of the comparison when sorting * Sorting for Top-K works. Probably slow. * Optimize pop-then-push into single operation * Fix lang::If to allow injection of custom then and else blocks * Added early exit block to consumer context to allow operators to quick exit * More comments * Fast-exit limits * Cleanup comments. Enabled parallel sorting. * Fixed bytecode compilation. Added explicit call handlers for sorter to bytecode builder. Fixed parallel sorting top-k * Set limit and offset to 0 when no limit/offset provided * Fix offset * Fix sort test in optimize to have well-defined order (some sort values were equal producing non-deterministic outputs). Fixed order by translator test to push a limit plan on top. This wasn't there before because we didn't need a limit * Cleanup * Fix limit without order-by clause

pmenon added enhancement do not merge labels Jun 27, 2018

pmenon requested a review from tcm-marcel June 27, 2018 12:24

tcm-marcel reviewed Jun 29, 2018

View reviewed changes

pmenon force-pushed the limit branch from 2923ce9 to fbb6be2 Compare July 1, 2018 03:24

pmenon force-pushed the limit branch from 1a4631d to ec37d5b Compare July 2, 2018 20:42

pmenon mentioned this pull request Jul 2, 2018

Limit without Order-By not working. #1451

Open

pmenon force-pushed the limit branch from ec37d5b to 8f1b6bc Compare July 3, 2018 02:18

pmenon added 16 commits July 20, 2018 18:51

Initial limit commit

8dbb983

Cleanup OrderByPlan

530349d

Don't use SQL types for the result of the comparison when sorting

245e427

Sorting for Top-K works. Probably slow.

2de0480

Optimize pop-then-push into single operation

8e3ff1e

Fix lang::If to allow injection of custom then and else blocks

f12925e

Added early exit block to consumer context to allow operators to quic…

7bab3c8

…k exit

More comments

d111ecd

Fast-exit limits

635ff58

Cleanup comments. Enabled parallel sorting.

d0ed5b3

Fixed bytecode compilation. Added explicit call handlers for sorter t…

d99e37b

…o bytecode builder. Fixed parallel sorting top-k

Set limit and offset to 0 when no limit/offset provided

3dcd084

Fix offset

497445a

Fix sort test in optimize to have well-defined order (some sort value…

f9a0972

…s were equal producing non-deterministic outputs). Fixed order by translator test to push a limit plan on top. This wasn't there before because we didn't need a limit

Cleanup

36e83bd

Fix limit without order-by clause

0e2de1e

pmenon force-pushed the limit branch from 8f1b6bc to 0e2de1e Compare July 20, 2018 23:00

pmenon added ready_for_review and removed do not merge labels Jul 22, 2018

tcm-marcel approved these changes Aug 2, 2018

View reviewed changes

Merge branch 'master' into limit

ad2b7b3

pmenon merged commit 1de8979 into cmu-db:master Aug 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support limit, offset, top-k serial and parallel versions in codegen #1435

Support limit, offset, top-k serial and parallel versions in codegen #1435

pmenon commented Jun 27, 2018

tcm-marcel left a comment

tcm-marcel Jun 29, 2018

pmenon Jul 1, 2018

tcm-marcel Jun 29, 2018

tcm-marcel Jun 29, 2018

pmenon Jul 1, 2018

tcm-marcel Jun 29, 2018 •

edited

Loading

pmenon Jul 2, 2018 •

edited

Loading

tcm-marcel Jun 29, 2018

pmenon Jul 1, 2018

tcm-marcel Jun 29, 2018

tcm-marcel Jun 29, 2018

pmenon Jul 1, 2018

coveralls commented Jul 1, 2018 •

edited

Loading

pmenon commented Jul 22, 2018

tcm-marcel left a comment

apavlo commented Aug 3, 2018

tcm-marcel commented Aug 3, 2018

apavlo commented Aug 4, 2018

crd477 commented Aug 6, 2018

crd477 commented Aug 7, 2018

Support limit, offset, top-k serial and parallel versions in codegen #1435

Support limit, offset, top-k serial and parallel versions in codegen #1435

Conversation

pmenon commented Jun 27, 2018

tcm-marcel 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

tcm-marcel Jun 29, 2018 • edited Loading

Choose a reason for hiding this comment

pmenon Jul 2, 2018 • edited Loading

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

coveralls commented Jul 1, 2018 • edited Loading

pmenon commented Jul 22, 2018

tcm-marcel left a comment

Choose a reason for hiding this comment

apavlo commented Aug 3, 2018

tcm-marcel commented Aug 3, 2018

apavlo commented Aug 4, 2018

crd477 commented Aug 6, 2018

crd477 commented Aug 7, 2018

tcm-marcel Jun 29, 2018 •

edited

Loading

pmenon Jul 2, 2018 •

edited

Loading

coveralls commented Jul 1, 2018 •

edited

Loading