Skip to content

Commit d98495b

Browse files
committed
module: Optimize RunModuleSplit().
This commit adds a level of indirection to the splits_ thread local variable: instead of keeping an entire batch for each possible output gate, we keep a pointer to a batch. This makes RunModuleSplit() faster, because we avoid copying the whole batch (we only use a pointer now). This also fixes Issue #443, because now we're using ~64K of stack space instead of ~2M. I tried running `samples/acl` on my laptop before and after the change and looking at how many packets are generated by source0: | source0 burst | before (Mpps) | after (Mpps) | | -------------:| -------------:| ------------:| | 32 | 76.0 | 83.2 | | 1 | 14.3 | 14.9 |
1 parent 08b9a3a commit d98495b

2 files changed

Lines changed: 20 additions & 17 deletions

File tree

core/module.cc

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -482,32 +482,33 @@ void Module::RunSplit(const gate_idx_t *out_gates,
482482
gate_idx_t pending[bess::PacketBatch::kMaxBurst];
483483
bess::PacketBatch batches[bess::PacketBatch::kMaxBurst];
484484

485-
bess::PacketBatch *splits = ctx.splits();
485+
bess::PacketBatch **splits = ctx.splits();
486486

487-
/* phase 1: collect unique ogates into pending[] */
487+
// phase 1: collect unique ogates into pending[] and add packets to local
488+
// batches, using splits to remember the association between an ogate and a
489+
// local batch
488490
for (int i = 0; i < cnt; i++) {
489491
bess::PacketBatch *batch;
490492
gate_idx_t ogate;
491493

492494
ogate = out_gates[i];
493-
batch = &splits[ogate];
495+
batch = splits[ogate];
496+
if (!batch) {
497+
batch = splits[ogate] = &batches[num_pending];
498+
batch->clear();
499+
pending[num_pending] = ogate;
500+
num_pending++;
501+
}
494502

495503
batch->add(*(p_pkt++));
496-
497-
pending[num_pending] = ogate;
498-
num_pending += (batch->cnt() == 1);
499504
}
500505

501-
/* phase 2: move batches to local stack, since it may be reentrant */
506+
// phase 2: clear splits, since it may be reentrant.
502507
for (int i = 0; i < num_pending; i++) {
503-
bess::PacketBatch *batch;
504-
505-
batch = &splits[pending[i]];
506-
batches[i].Copy(batch);
507-
batch->clear();
508+
splits[pending[i]] = nullptr;
508509
}
509510

510-
/* phase 3: fire */
511+
// phase 3: fire
511512
for (int i = 0; i < num_pending; i++)
512513
RunChooseModule(pending[i], &batches[i]);
513514
}

core/worker.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,7 @@ class Worker {
9191
gate_idx_t current_igate() const { return current_igate_; }
9292
void set_current_igate(gate_idx_t idx) { current_igate_ = idx; }
9393

94-
/* better be the last field. it's huge */
95-
bess::PacketBatch *splits() { return splits_; }
94+
bess::PacketBatch **splits() { return splits_; }
9695

9796
private:
9897
volatile worker_status_t status_;
@@ -115,8 +114,11 @@ class Worker {
115114
* Modules should use get_igate() for access */
116115
gate_idx_t current_igate_;
117116

118-
/* better be the last field. it's huge */
119-
bess::PacketBatch splits_[MAX_GATES + 1];
117+
// For each possible output gate contains a pointer to a batch, or nullptr,
118+
// if no batch has been associated with the output gate yet.
119+
//
120+
// This should be the last field, since it's huge.
121+
bess::PacketBatch *splits_[MAX_GATES + 1];
120122
};
121123

122124
// NOTE: Do not use "thread_local" here. It requires a function call every time

0 commit comments

Comments
 (0)