Skip to content

Conversation

@drahnr
Copy link
Contributor

@drahnr drahnr commented Feb 2, 2026

Uses a static rocksdb library (rocksdb.a on Linux, we only build on Linux), except for publish-related CI runs.

Why

We spend significant time on building the same artifact repeatedly, and

What

  1. small fix for recently introduced binaries that didn't have the C++ std lib linkage fix setup correctly
  2. fix for test runs to also include C++ std lib where relevant (via miden-node-util)
  3. adds a GHA workflow to build a rocksdb.a static library, and use that in conjunction with the "right" env vars to skip build - we also skip caching rocksdb (C++) build artifacts which saves us a good chunk of cache, once used uniformly.

Open questions:

  1. since build using cargo b -p librocksdb-sys@{{ version }} --locked we will build the exact same library, so we might want to unify the approach to publish-* workflows as well - I think we should

@drahnr drahnr added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Feb 2, 2026
@drahnr drahnr force-pushed the bernhard-rocksdb-ci-impro branch 2 times, most recently from 4e083ee to 3ef9d51 Compare February 3, 2026 16:38
@drahnr
Copy link
Contributor Author

drahnr commented Feb 5, 2026

@Mirko-von-Leipzig this is going to conflict quite a bit with your refactor into a single file (in a "good way" in a sense)

@Mirko-von-Leipzig
Copy link
Collaborator

Mirko-von-Leipzig commented Feb 6, 2026

@Mirko-von-Leipzig this is going to conflict quite a bit with your refactor into a single file (in a "good way" in a sense)

How would you like to go about it? I think after mine makes sense because it simplifies your diff here iiuc? But of course I'm biased 😁 I don't mind if you'd prefer to go ahead with this first.

We could also forgo this PR entirely and see whether the caching changes are dramatic enough to not care about this?

@drahnr
Copy link
Contributor Author

drahnr commented Feb 6, 2026

Please merge your's first, I'll do some comparison of cache+time afterwards

@drahnr drahnr force-pushed the bernhard-rocksdb-ci-impro branch from 6384ded to 1c0af0d Compare February 9, 2026 10:10
@drahnr drahnr marked this pull request as ready for review February 9, 2026 19:00
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this is still worth adding to CI runs? Right now all large jobs here build on top of the build job which should already mean compiling this only once due to shared caching.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd safe on link-time, but there is complexity penalty adding.
Haven't checked the "savings" if any just yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog This PR does not require an entry in the `CHANGELOG.md` file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants