Skip to content

Commit 49d4a85

Browse files
loganpowellloganpowell
committed
style: clang-format all files changed by feat/concurrent-writes
1 parent 201fb77 commit 49d4a85

19 files changed

Lines changed: 149 additions & 140 deletions

‎.github/workflows/concurrent-writes-ci.yml‎

Lines changed: 80 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ name: ladybug concurrent-writes CI
33
# Produces:
44
# - JSONL test results (one record per test case)
55
# - Markdown test summary → GitHub Step Summary
6-
# - Before/after benchmark JSON files
6+
# - Before/after benchmark JSON files (both built on the same ubuntu runner)
77
# - Benchmark comparison → PR comment + GitHub Step Summary
88

99
on:
@@ -20,7 +20,7 @@ concurrency:
2020
cancel-in-progress: true
2121

2222
jobs:
23-
# ─── Build ────────────────────────────────────────────────────────────────
23+
# ─── Build (branch) ───────────────────────────────────────────────────────
2424
build:
2525
name: Build (${{ matrix.os }})
2626
runs-on: ${{ matrix.os }}
@@ -67,6 +67,50 @@ jobs:
6767
path: build/relwithdebinfo
6868
retention-days: 1
6969

70+
# ─── Build main (benchmark baseline) ─────────────────────────────────────
71+
# Runs in parallel with the branch build. The build output is cached across
72+
# workflow runs (main rarely changes) so this is typically ~1 min after the
73+
# first run.
74+
build-main:
75+
name: Build main (benchmark baseline)
76+
runs-on: ubuntu-latest
77+
if: github.event_name == 'pull_request' || github.ref == 'refs/heads/feat/concurrent-writes'
78+
79+
steps:
80+
- uses: actions/checkout@v4
81+
with:
82+
ref: main
83+
84+
- name: Install dependencies
85+
run: |
86+
sudo apt-get update -qq
87+
sudo apt-get install -y cmake ninja-build
88+
89+
- name: Cache CMake build (main)
90+
uses: actions/cache@v4
91+
with:
92+
path: build
93+
key: build-main-ubuntu-latest-${{ hashFiles('CMakeLists.txt', 'src/**/*.h') }}
94+
restore-keys: build-main-ubuntu-latest-
95+
96+
- name: Configure (CMake + Ninja)
97+
run: |
98+
cmake -B build/relwithdebinfo -G Ninja \
99+
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
100+
-DBUILD_TESTS=TRUE \
101+
-DENABLE_BACKTRACES=TRUE \
102+
.
103+
104+
- name: Build
105+
run: cmake --build build/relwithdebinfo --parallel 4
106+
107+
- name: Upload main build
108+
uses: actions/upload-artifact@v4
109+
with:
110+
name: ladybug-main-build-ubuntu-latest
111+
path: build/relwithdebinfo
112+
retention-days: 1
113+
70114
# ─── Test ─────────────────────────────────────────────────────────────────
71115
test:
72116
name: Test (${{ matrix.os }})
@@ -132,50 +176,38 @@ jobs:
132176

133177
# ─── Benchmark ────────────────────────────────────────────────────────────
134178
#
135-
# Produces a before/after comparison:
136-
#
137-
# baseline.json — captured on main (unpatched ladybug)
138-
# committed to the repo; CI reads it if present
139-
# branch-results.json — captured on this branch
140-
# comparison.md — diff table posted as PR comment + Step Summary
141-
#
142-
# To capture a new baseline locally (e.g. after a significant ladybug update):
143-
#
144-
# git checkout main
145-
# make relwithdebinfo # or: cmake ... && cmake --build ...
146-
# python3 benchmarks/run_benchmarks.py \
147-
# --build-dir build/relwithdebinfo \
148-
# --dataset-dir dataset \
149-
# --output benchmarks/baseline.json --label main
179+
# Both main and feat/concurrent-writes are built on the same ubuntu-latest
180+
# runner (in the build and build-main jobs above) so the comparison is
181+
# apples-to-apples regardless of local hardware.
150182
#
151-
# # Also archive to results/ for posterity:
152-
# cp benchmarks/baseline.json \
153-
# benchmarks/results/main-$(git rev-parse --short HEAD)-$(date +%Y%m%d).json
154-
#
155-
# git add benchmarks/baseline.json benchmarks/results/ \
156-
# && git commit -m "bench: capture baseline"
157-
#
158-
# # Generate a history report:
159-
# python3 benchmarks/report.py --output benchmarks/results/report.md
183+
# The main build is heavily cached — after the first run it typically
184+
# restores in ~1 min. The branch build is downloaded as a pre-built artifact.
160185
#
161186
benchmark:
162187
name: Benchmark (ubuntu-latest)
163-
needs: build
188+
needs: [build, build-main]
164189
runs-on: ubuntu-latest
165190
if: github.event_name == 'pull_request' || github.ref == 'refs/heads/feat/concurrent-writes'
166191

167192
steps:
168193
- uses: actions/checkout@v4
169194

170-
- name: Download build (ubuntu)
195+
- name: Download branch build (ubuntu)
171196
uses: actions/download-artifact@v4
172197
with:
173198
name: ladybug-build-ubuntu-latest
174199
path: build/relwithdebinfo
175200

201+
- name: Download main build (ubuntu)
202+
uses: actions/download-artifact@v4
203+
with:
204+
name: ladybug-main-build-ubuntu-latest
205+
path: build-main/relwithdebinfo
206+
176207
- name: Restore binary permissions
177208
run: |
178-
find build/relwithdebinfo -type f | xargs chmod +x 2>/dev/null || true
209+
find build/relwithdebinfo -type f | xargs chmod +x 2>/dev/null || true
210+
find build-main/relwithdebinfo -type f | xargs chmod +x 2>/dev/null || true
179211
180212
- name: Fetch dataset (shallow)
181213
run: |
@@ -186,27 +218,32 @@ jobs:
186218
with:
187219
python-version: "3.11"
188220

221+
- name: Benchmark main (in-runner baseline)
222+
run: |
223+
git fetch --depth=1 origin main
224+
MAIN_SHA=$(git rev-parse --short FETCH_HEAD)
225+
python3 benchmarks/run_benchmarks.py \
226+
--build-dir build-main/relwithdebinfo \
227+
--dataset-dir dataset \
228+
--output benchmarks/ci-baseline.json \
229+
--label "main (${MAIN_SHA})"
230+
189231
- name: Run benchmarks (branch)
190232
run: |
191233
python3 benchmarks/run_benchmarks.py \
192234
--build-dir build/relwithdebinfo \
193235
--dataset-dir dataset \
194236
--output benchmarks/branch-results.json \
195-
--label "${{ github.head_ref || github.ref_name }}"
237+
--label "${{ github.head_ref || github.ref_name }} ($(git rev-parse --short HEAD))"
196238
197-
- name: Compare against baseline
239+
- name: Compare branch vs main (same runner)
198240
id: compare
199241
run: |
200-
if [ -f benchmarks/baseline.json ]; then
201-
python3 benchmarks/compare_benchmarks.py \
202-
--baseline benchmarks/baseline.json \
203-
--branch benchmarks/branch-results.json \
204-
--output benchmarks/comparison.md
205-
echo "has_baseline=true" >> "$GITHUB_OUTPUT"
206-
else
207-
python3 -c "open('benchmarks/comparison.md','w').write('## \u26a1 Concurrent-write benchmark comparison\n\n> **No baseline found.** To capture one, run on \`main\`:\n> \`\`\`\n> python3 benchmarks/run_benchmarks.py \\\\\n> --build-dir build/relwithdebinfo \\\\\n> --dataset-dir dataset \\\\\n> --output benchmarks/baseline.json --label main\n> \`\`\`\n> Then commit \`benchmarks/baseline.json\` to the repo.\n')"
208-
echo "has_baseline=false" >> "$GITHUB_OUTPUT"
209-
fi
242+
python3 benchmarks/compare_benchmarks.py \
243+
--baseline benchmarks/ci-baseline.json \
244+
--branch benchmarks/branch-results.json \
245+
--output benchmarks/comparison.md
246+
echo "has_baseline=true" >> "$GITHUB_OUTPUT"
210247
211248
- name: Write benchmark comparison to Step Summary
212249
run: cat benchmarks/comparison.md >> $GITHUB_STEP_SUMMARY
@@ -245,10 +282,9 @@ jobs:
245282
--output benchmarks/results/report.md 2>/dev/null || true
246283
247284
- name: Fail CI if regressions detected
248-
if: steps.compare.outputs.has_baseline == 'true'
249285
run: |
250286
python3 benchmarks/compare_benchmarks.py \
251-
--baseline benchmarks/baseline.json \
287+
--baseline benchmarks/ci-baseline.json \
252288
--branch benchmarks/branch-results.json
253289
254290
- name: Upload benchmark artifacts
@@ -257,6 +293,7 @@ jobs:
257293
with:
258294
name: benchmark-results
259295
path: |
296+
benchmarks/ci-baseline.json
260297
benchmarks/branch-results.json
261298
benchmarks/comparison.md
262299
benchmarks/results/

‎src/catalog/catalog.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -641,8 +641,8 @@ void Catalog::serialize(Serializer& ser) const {
641641
}
642642

643643
void Catalog::serializeSnapshot(Serializer& ser, common::transaction_t snapshotTS) const {
644-
const Transaction snapshotTxn(TransactionType::CHECKPOINT,
645-
Transaction::DUMMY_TRANSACTION_ID, snapshotTS);
644+
const Transaction snapshotTxn(TransactionType::CHECKPOINT, Transaction::DUMMY_TRANSACTION_ID,
645+
snapshotTS);
646646
tables->serializeSnapshot(ser, &snapshotTxn);
647647
sequences->serializeSnapshot(ser, &snapshotTxn);
648648
functions->serializeSnapshot(ser, &snapshotTxn);

‎src/catalog/catalog_set.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,7 @@ void CatalogSet::serialize(Serializer serializer) const {
266266
}
267267
}
268268

269-
void CatalogSet::serializeSnapshot(Serializer serializer,
270-
const Transaction* snapshotTxn) const {
269+
void CatalogSet::serializeSnapshot(Serializer serializer, const Transaction* snapshotTxn) const {
271270
std::shared_lock lck{mtx};
272271
std::vector<CatalogEntry*> entriesToSerialize;
273272
for (auto& [_, entry] : entries) {
@@ -281,8 +280,7 @@ void CatalogSet::serializeSnapshot(Serializer serializer,
281280
case CatalogEntryType::FOREIGN_TABLE_ENTRY:
282281
continue;
283282
default: {
284-
auto visibleEntry =
285-
traverseVersionChainsForTransactionNoLock(snapshotTxn, entry.get());
283+
auto visibleEntry = traverseVersionChainsForTransactionNoLock(snapshotTxn, entry.get());
286284
if (visibleEntry && !visibleEntry->isDeleted()) {
287285
entriesToSerialize.push_back(visibleEntry);
288286
}

‎src/common/file_system/file_system.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
#include "common/file_system/file_system.h"
22

3-
#include <format>
4-
53
#include "common/exception/io.h"
64
#include "common/string_utils.h"
5+
#include <format>
76

87
namespace lbug {
98
namespace common {
@@ -16,8 +15,8 @@ void FileSystem::renameFile(const std::string& from, const std::string& to) {
1615
std::error_code ec;
1716
std::filesystem::rename(from, to, ec);
1817
if (ec) {
19-
throw IOException(std::format("Error renaming file {} to {}. ErrorMessage: {}", from, to,
20-
ec.message()));
18+
throw IOException(
19+
std::format("Error renaming file {} to {}. ErrorMessage: {}", from, to, ec.message()));
2120
}
2221
}
2322

‎src/common/file_system/local_file_system.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ std::unique_ptr<FileInfo> LocalFileSystem::openFile(const std::string& path, Fil
135135
throw IOException(std::format("Cannot open file {}: {}", fullPath, posixErrMessage()));
136136
}
137137
if (flags.lockType != FileLockType::NO_LOCK) {
138-
struct flock fl {};
138+
struct flock fl{};
139139
memset(&fl, 0, sizeof fl);
140140
fl.l_type = flags.lockType == FileLockType::READ_LOCK ? F_RDLCK : F_WRLCK;
141141
fl.l_whence = SEEK_SET;
@@ -197,8 +197,8 @@ void LocalFileSystem::renameFile(const std::string& from, const std::string& to)
197197
std::error_code ec;
198198
std::filesystem::rename(from, to, ec);
199199
if (ec) {
200-
throw IOException(std::format("Error renaming file {} to {}. ErrorMessage: {}", from, to,
201-
ec.message()));
200+
throw IOException(
201+
std::format("Error renaming file {} to {}. ErrorMessage: {}", from, to, ec.message()));
202202
}
203203
}
204204

@@ -578,7 +578,7 @@ uint64_t LocalFileSystem::getFileSize(const FileInfo& fileInfo) const {
578578
}
579579
return size.QuadPart;
580580
#else
581-
struct stat s {};
581+
struct stat s{};
582582
if (fstat(localFileInfo->fd, &s) == -1) {
583583
throw IOException(std::format("Cannot read size of file. path: {} - Error {}: {}",
584584
fileInfo.path, errno, posixErrMessage()));

‎src/include/catalog/catalog.h‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -227,12 +227,8 @@ class LBUG_API Catalog {
227227
bool changedSinceLastCheckpoint() const {
228228
return version.load(std::memory_order_relaxed) != lastCheckpointVersion;
229229
}
230-
void resetVersion() {
231-
lastCheckpointVersion = version.load(std::memory_order_relaxed);
232-
}
233-
void resetVersion(uint64_t checkpointedVersion) {
234-
lastCheckpointVersion = checkpointedVersion;
235-
}
230+
void resetVersion() { lastCheckpointVersion = version.load(std::memory_order_relaxed); }
231+
void resetVersion(uint64_t checkpointedVersion) { lastCheckpointVersion = checkpointedVersion; }
236232

237233
void setCatalogName(const std::string& name) { catalogName = name; }
238234
std::string getCatalogName() const { return catalogName; }

‎src/include/storage/page_manager.h‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,8 @@ class PageManager : public PageAllocator {
2828
bool changedSinceLastCheckpoint() const {
2929
return version.load(std::memory_order_relaxed) != lastCheckpointVersion;
3030
}
31-
void resetVersion() {
32-
lastCheckpointVersion = version.load(std::memory_order_relaxed);
33-
}
34-
void resetVersion(uint64_t checkpointedVersion) {
35-
lastCheckpointVersion = checkpointedVersion;
36-
}
31+
void resetVersion() { lastCheckpointVersion = version.load(std::memory_order_relaxed); }
32+
void resetVersion(uint64_t checkpointedVersion) { lastCheckpointVersion = checkpointedVersion; }
3733

3834
PageRange allocatePageRange(common::page_idx_t numPages) override;
3935
void freePageRange(PageRange block) override;

‎src/include/storage/table/column_chunk.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ struct ChunkCheckpointState {
3131
struct SegmentCheckpointState {
3232
SegmentCheckpointState(const ColumnChunkData& chunkData, common::row_idx_t startRowInData,
3333
common::row_idx_t offsetInSegment, common::row_idx_t numRows)
34-
: chunkData{chunkData}, startRowInData{startRowInData},
35-
offsetInSegment{offsetInSegment}, numRows{numRows} {}
34+
: chunkData{chunkData}, startRowInData{startRowInData}, offsetInSegment{offsetInSegment},
35+
numRows{numRows} {}
3636

3737
const ColumnChunkData& chunkData;
3838
common::row_idx_t startRowInData;

‎src/include/storage/table/node_table.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,7 @@ class LBUG_API NodeTable : public Table {
187187
void commit(main::ClientContext* context, catalog::TableCatalogEntry* tableEntry,
188188
LocalTable* localTable) override;
189189
bool checkpoint(main::ClientContext* context, catalog::TableCatalogEntry* tableEntry,
190-
PageAllocator& pageAllocator,
191-
const transaction::Transaction* snapshotTxn = nullptr,
190+
PageAllocator& pageAllocator, const transaction::Transaction* snapshotTxn = nullptr,
192191
uint64_t epochWatermark = 0) override;
193192
void rollbackCheckpoint() override;
194193
void reclaimStorage(PageAllocator& pageAllocator) const override;

‎src/include/storage/table/rel_table.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,7 @@ class LBUG_API RelTable : public Table {
168168
}
169169
common::column_id_t getNumColumns() const {
170170
DASSERT(directedRelData.size() >= 1);
171-
RUNTIME_CHECK(for (const auto& relData
172-
: directedRelData) {
171+
RUNTIME_CHECK(for (const auto& relData : directedRelData) {
173172
DASSERT(relData->getNumColumns() == directedRelData[0]->getNumColumns());
174173
});
175174
return directedRelData[0]->getNumColumns();
@@ -184,8 +183,7 @@ class LBUG_API RelTable : public Table {
184183
void commit(main::ClientContext* context, catalog::TableCatalogEntry* tableEntry,
185184
LocalTable* localTable) override;
186185
bool checkpoint(main::ClientContext*, catalog::TableCatalogEntry* tableEntry,
187-
PageAllocator& pageAllocator,
188-
const transaction::Transaction* snapshotTxn = nullptr,
186+
PageAllocator& pageAllocator, const transaction::Transaction* snapshotTxn = nullptr,
189187
uint64_t epochWatermark = 0) override;
190188
void rollbackCheckpoint() override {};
191189
void reclaimStorage(PageAllocator& pageAllocator) const override;

0 commit comments

Comments
 (0)