It would be nice if we could figure out why macOS SQLite is different from self built so that we can replicate those performance improvements in our depends build.

Concept ACK

The Homebrew docs recommend to "not remove macOS native tools and forcefully replace them with symlinks back to the Homebrew-provided tool."

But I couldn't find anything on whether MacOS's version of SQLite is different to the brew version of SQLite (assuming the same version numbers). That difference in performance is surely not entirely explained by different version numbers being used.

Concept ACK

it's not correct to say there was "no rationale", it's in the pr description:

On master (7ae86b3) installed Homebrew sqlite package is ignored during build on macOS.
This PR fixes this issue and update macOS build docs.

But we now know we'd prefer not to use the brew version.

It would be nice if we could figure out why macOS SQLite is different from self built so that we can replicate those performance improvements in our depends build.

I agree. Started having a look through https://github.com/apple-oss-distributions to see if there might be any clues.

it's not correct to say there was "no rationale", it's in the pr description:

The description just says we should prefer brew installed sqlite if it happens to be installed. There's no reasoning given as to why we should prefer that instead of just using the system library (i.e it's faster, has newer features we require, system libs are severely outdated/buggy) etc.

What is the sqlite version on macOS 10.15?

Tested d216d71

On the first machine the system library is twice as fast, on the second machine there's no difference (or it's even slightly slower). I checked in the log that right Sqlite version was indeed used.

I agree with @hebasto that we should check if this holds for older (supported) macOS versions. Perhaps a safer approach is to pick whichever version is higher.

Machine 1 MacBook Pro (2.3 GHz 8-Core Intel Core i9), macOS 12.5.1

• with Sqlite 3.39.2 via homebrew: 3.8s
• system Sqlite 3.37.0: 2.0s

Machine 2: iMac (3.4 GHz Quad-Core Intel Core i5): macOS 12.5.1

• with Sqlite 3.39.2 via homebrew: 0.5s
• system Sqlite 3.37.0: 0.6s

Note that machine 2 is much slower than machine 1, yet sqlite performs faster. There's something else going on, possibly specific to my setup. I got even more extreme results with the migratewallet RPC.

Perhaps a safer approach is to pick whichever version is higher.

NACK. More often than not, that would just result in the situation where we get worse performance. We aren't using features from newer versions, and Apple already backports bug and security fixes to the system libs.

What is the sqlite version on macOS 10.15?

Looks like it is 3.28.0+.

So this is preferring the macOS/native sqlite over the Homebrew sqlite, right?
(I assume we already prefer depends-built if it's been made available.)

Can the user still explicitly override it and build with Homebrew's if they so choose? (--with-sqlite=/some/path?)

So this is preferring the macOS/native sqlite over the Homebrew sqlite, right?

Yes. See also #25724.

(I assume we already prefer depends-built if it's been made available.)

If by "made available", you mean a CONFIG_SITE/--prefix was passed to configure, then yes.

Can the user still explicitly override it and build with Homebrew's if they so choose?

Yes. The user can ultimately still choose whichever sqlite they prefer. i.e set LDFLAGS & CPPFLAGS to point to brews sqlite, and that will be used.

Not sure if this issue applies to arm64 (intel only) or if I'm testing incorrectly. Installed sqlite3 from homebrew and I'm not seeing a difference between master and this branch on a MacBook Pro 16 (M1 Max), macOS 12.3. In both cases the example takes 5 seconds. Tested back and forth a few times.

$  clang -v
Apple clang version 13.1.6 (clang-1316.0.21.2.5)
Target: arm64-apple-darwin21.4.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

$ ./configure --with-incompatible-bdb --enable-suppress-external-warnings CC=clang CXX=clang++

$ ./src/bitcoin-cli -rpcwallet=speedy importdescriptors   0.00s user 0.00s system 0% cpu 5.293 total

$ sqlite3 --version
3.37.0 2021-12-09 01:34:53 9ff244ce0739f8ee52a3e9671adb4ee54c83c640b02e3f9d185fd2f9a179aapl

Ah, the system sqlite3 3.37 was being used in either case. Needed to restart the terminal to pick up the link (echo 'export PATH="/opt/homebrew/opt/sqlite/bin:$PATH"' >> ~/.zshrc) to sqlite3 3.39.3 2022-09-05, after which both master and this branch run the example rpc after configure + make in around half a second (macOS Monterey 12.3).

Just tried (the latest master) with macOS 10.15 Catalina, freshly installed on a 2015 Macbook Pro, Intel i5 dual core:

• with Sqlite 3.39.3 via homebrew: 1.6s
• system Sqlite 3.28.0: 1.6s

(caveat: I only synced a fraction of mainnet and then paused it, so the rescan part is a bit faster: 1ms on the 2015 machine vs 423ms on the 2019 machine)

I'm not sure what to make of the benchmarking / results being posted here. I think in some cases not quite the right thing is being tested, or results are being mixed up. However the general case still seems to be that the brew libs are either the same speed, or much slower than the system libs.

I haven't seen an argument for always prefering the brew libs, and in that case you're just rolling the dice for much worse performance, and ending up with #25724.

Note that the benchmarking can also be performed without using brew at all. You can just compare a build with system libs, vs a depends build, as sqlite as compiled via brew, is going to be ~the same as the lib you get when compiling in depends.

Looking at the macOS sqlite system lib, it's pretty clear that it contains code that isn't in the open source sqlite. There are a number of additional symbols, including __sqlite3_apple_archive, __sqlite3_apple_archive_type, __sqlite3_apple_unarchive, __sqlite3_purgeEligiblePagerCacheMemory etc. As well as _sqlite3_key, _sqlite3_key_v2, which indicate the presence of non-free sqlite extensions, such as The SQLite Encryption Extension.

However the general case still seems to be that the brew libs are either the same speed, or much slower than the system libs.

That's what I was testing.

Concept ACK

The presence of a Homebrew version doesn't mean the user intended it for Bitcoin. Relying on closed source non free Apple stuff that's sometimes faster, but in (so far) unpredictable ways, doesn't seem very appealing either.

But the fact that the non-homebrew version sometimes is dramatically faster, is probably a good enough reason to prefer it.

This seems to fix my performance issues on my m1 mac when running tests against bitcoind.

This is on the official v23 release (36 seconds)

This is on d216d71 (!5 seconds!)

So this is a dramatic performance improvement for our test harness!

In case anyone wants stack exchange reputation, consider answering this question: https://bitcoin.stackexchange.com/questions/113898/bitcoin-v23-is-10-times-slower-than-v22-on-macos-for-basic-regtest-tests

This seems to fix my performance issues on my m1 mac when running tests against bitcoind.

Thanks for confirming this fixes the performance issues you've been seeing. While the release binaries still wont be "fixed", if/when this is merged, the situation for compiling locally / development should be much improved.

In case anyone wants stack exchange reputation, consider answering this question: https://bitcoin.stackexchange.com/questions/113898/bitcoin-v23-is-10-times-slower-than-v22-on-macos-for-basic-regtest-tests

I'm mostly interested in running integration tests for a project and it looks like I cannot upgrade to the new version yet without solving this.

Good to know that they were using x86_64, so we can further rule out an x86_64 vs arm64 (M1) difference. The fact that developers are landing on the solution of "not upgrading", is another reason to fix this.

ACK d216d71

I also observe a significant performance improvement with this.

This reverts ee7b84e from #20527.
That change was made without any rationale

That is untrue. Besides #20527 description. one could refer #20498. Build docs implied that Bitcoin Core uses Homebrew's sqlite package. And #20527 made it explicit.

I'd say the opposite: this reversion is unjustified.

1. Relying on Apple's sqlite package undermines Bitcoin Core security.

when building from source on macOS, it just results in drastically worse performance

2. Then such performance issue should be fixed in Homebrew's sqlite package.

The user can ultimately still choose whichever sqlite they prefer. i.e set LDFLAGS & CPPFLAGS to point to brews sqlite, and that will be used.

3. If users want to use Apple's sqlite, they can use the same approach. No code change is required at all.

NACK from me.

Besides #20527 description. one could refer #20498.

There is nothing in either of these threads that gives reasoning for why a brew installed sqlite should be preferred. #20498 just seems to be figuring out if macOS comes with a sqlite system library.

Relying on Apple's sqlite package undermines Bitcoin Core security.

How so? This isn't being used in the release binaries. It's for developers/users self-compiling on macos. Self compilation / developer envinronments already come with lessened security assumptions, because your dependencies are coming from third party package managers, the OS, etc.

Then such performance issue should be fixed in Homebrew's sqlite package.

What you mean? Homebrew just compiles the unmodified sqlite source, there is nothing to "fix" there. Fixing this likely requires upstreaming changes to sqlite. If we actually had those changes, we could also be applying them in depends (and fixing this for releases).

If users want to use Apple's sqlite, they can use the same approach. No code change is required at all.

So you suggest we leave this as-is, and just continually tell downstream projects / confused devs that to get reasonable performance in their developer environments, they have to compile differently, and avoid homebrews sqlite, even though our build system is setup to prefer it by default?

Here's another instance of a project, this time BDK, running into this same problem: bitcoindevkit/bdk#749.

Relying on Apple's sqlite package undermines Bitcoin Core security.

How so?

Like that:

Looking at the macOS sqlite system lib, it's pretty clear that it contains code that isn't in the open source sqlite.

If users want to use Apple's sqlite, they can use the same approach. No code change is required at all.

So you suggest we leave this as-is, and just continually tell downstream projects / confused devs that to get reasonable performance in their developer environments, they have to compile differently, and avoid homebrews sqlite, even though our build system is setup to prefer it by default?

Our defaults must prioritize security, not performance.

To avoid confusing, using of an Apple's library can be provided as an example in our docs.

Like that:

Can you clarify what you mean by security? I don't quite understand why we must avoid the system sqlite lib at all costs, while at the same time we are going to happily load any other number of apple shared libs (which are complete black boxes) at runtime (also true for release binaries).

Our defaults must prioritize security, not performance.
To avoid confusing, using of an Apple's library can be provided as an example in our docs.

In our release binaries, I agree. However, self-compilation on macOS, is almost always going to be a developer environment, which as I mentioned above, already comes with lessened security assumptions.

In this case, from a security perspective, why is using Apples sqlite dylib, any worse than using homebrews? In both cases, you're using a precompiled library, provided by a third party. Are homebrews prebuilt libs even deterministic/reproducible? (I know you can tell brew to build from source, but 99.9% of users installing sqlite from brew are not doing that)?

Possibly related: https://bonsaidb.io/blog/acid-on-apple/

Possibly related: https://bonsaidb.io/blog/acid-on-apple/

Aha! I think this describes the problem exactly. Disabling fullfsync gets us the performance of using system sqlite. The question is whether it is safe for us to do that.

Aha! I think this describes the problem exactly. Disabling fullfsync gets us the performance of using system sqlite. The question is whether it is safe for us to do that.

It's quite possible that Apple is making assumptions about sqlite & the OS, that allows it to make these modifications / other improvements, that we likely cannot. In any case, we should still want this change, (assuming we could come up with a safe patchset for depends (for 25.0)), otherwise by picking brews sqlite, we would then still be choosing the worst of 3 available options, for performance, and continuing to cause confusion / issues downstream.

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

NACK from me.

After having an offline discussion with @fanquake, I agree with his points.

Concept ACK.

Our docs say:

sqlite is required to support for descriptor wallets.

macOS ships with a useable sqlite package, meaning you don't need to install anything.

But Homebrew can install its sqlite package as an other package's dependency, and next time during configuring another sqlite library will be used with no user's intention. And this PR prevents such cases.

There is no reason to assume these are developer environments

I agree with this point. Although most users won't self compile, I would not assume that most people who self compile are (experienced) developers.

I don't quite understand why we must avoid the system sqlite lib at all costs, while at the same time we are going to happily load any other number of apple shared libs (which are complete black boxes) at runtime (also true for release binaries).

I also agree with this. If (a rogue) Apple (employee) wanted to steal coins, there are probably more obscure corners of their black box library to put the backdoor in than SQlite.

Is ~3.4s really so bad for importing a bunch of descriptors?

It's actually worse that this, migrating a legacy wallet to descriptors takes more than ten minutes for me and leaves the wallet in an incorrect state. That said, this PR actually doesn't fix that problem for me, but it might for others.

I do hope we can fix the real problem soon(tm), it seems @achow101 is optimistic.

I encountered the issue when working with BDK.
This fixed the issue for me. Previously it took ~1 minute to complete, now it takes ~3 seconds.

OS: Mac OS Monterey 12.6
Processor: Intel Core i5
SQLite Version 3.37.0

Would be nice if this is back ported on the effected version

It can be merged into 24.x as-is (#26327), but for the other branches, someone will need to cherry-pick.

Did a cherry-pick for 23.x in #26333.

22.x in #26350.