macOS: disable AppNap during sync #12783

krab · 2018-03-26T05:40:01Z

Code based on pull/5804. Tested only on macOS 10.13.3 and should support 10.9+.

What macOS versions bitcoin core currently supports?

Empact · 2018-03-26T05:43:10Z

jonasschnelli · 2018-03-26T07:57:10Z

src/support/macos_appnap.mm

+class CAppNapInhibitorInt
+{
+public:
+    id<NSObject> activityId;


Is there a difference in using NSObject *activityId?

jonasschnelli · 2018-03-26T07:57:31Z

src/support/macos_appnap.mm

+    ~(NSActivitySuddenTerminationDisabled |
+      NSActivityAutomaticTerminationDisabled);
+
+    if ([[NSProcessInfo processInfo] respondsToSelector:@selector(beginActivityWithOptions:reason:)])


nit: code styling, use brackets {}

jonasschnelli · 2018-03-26T07:59:10Z

src/support/macos_appnap.mm

+    if ([[NSProcessInfo processInfo] respondsToSelector:@selector(endActivity:)])
+        [[NSProcessInfo processInfo] endActivity:priv->activityId];
+#endif
+    delete priv;


Since we are only holding a NSObject pointer, is this deallocation really required or could it even be messing with the autorelease pool?

jonasschnelli · 2018-03-26T07:59:39Z

utACK e41bb4a4568e08fe75ab8af7de7db1a2684cd989

Sjors · 2018-03-27T08:36:39Z

Will review later. Good question regarding which macOS version to support. The release notes state 10.8+, but I wonder how true that is. Apple pushes macOS updates quite aggressively and the lack of VM's make downgrades impractical. So unless any of the active devs here still runs an older version, I don't think it's realistic to support anything but the current macOS release and maybe one earlier version .

jonasschnelli · 2018-03-27T08:41:42Z

We do currently support 10.8+ and I have VMs for all OSX versions from 10.5 upwards. I do test the RC regularly in 10.8 till 10.x VMs.

Sjors · 2018-03-27T09:00:16Z

@jonasschnelli ok, that's great to know. Any hints for setting up a 10.8 VM on an iMac?

laanwj · 2018-03-27T17:01:34Z

Concept ACK.
Should appnap always be disabled, or just during initial sync? It's something that could be done later, but worth thinking about I think.

theuni · 2018-03-28T22:03:18Z

I really really don't like the idea of adding a new compiler requirement for bitcoind (which is why we closed #5804).

How about using this for bitcoin-qt only ?

Sjors · 2018-03-29T10:44:38Z

I think it's reasonable* to expect bitcoind users to understand their OS power management.

= though not always true :-)

krab · 2018-03-31T16:55:55Z

how to i squash all my commits in PR they contain merging commits from upstream/master 😕

krab · 2018-03-31T18:28:22Z

Squashed all my commits into single one.

OS X Mountain Lion 10.8 not supported by Apple anymore. Mozilla/Google don't support 10.8 either in their web browsers.
Qt 5.7 (currently used in bitcoin-qt0.16.0) support expired in June 15, 2017.
Qt 5.8 supports only 10.9+.
Qt 5.9 only 10.10+.
So OS X 10.8 support in bitcoin-qt pretty much pointless since it requires Qt 5.6/5.7 and very old XCode to build.

laanwj · 2018-04-07T13:51:32Z

How about using this for bitcoin-qt only ?

Would make sense to me. Would (for me) be preferable to having objective C in the core code.

I think it's reasonable* to expect bitcoind users to understand their OS power management.

Virtually no one uses bitcoind on MacOSX. For a long time, we didn't even build it, and there was about one person that requested it :)

theuni · 2018-04-09T18:11:23Z

src/qt/macos_appnap.mm

+
+CAppNapInhibitor::CAppNapInhibitor(const char* strReason) : d(new Private)
+{
+    const NSActivityOptions activityOptions =


Since this isn't defined for < MAC_OS_X_VERSION_10_9, does it need a runtime check?

No idea, i can't setup 10.8 build vm to test code properly, so pre 10.9 support removed.

fanquake · 2018-04-12T01:16:18Z

Started doing some testing.
Using master (979f598) I can make bitcoin-qt and Bitcoin Core (app bundle) enter app nap quite quickly just by starting and then minimising the app window.
You can check App Nap status using Activity Monitor -> Energy.

Using c975861, neither process seems to go into App Nap:

theuni · 2018-04-12T18:48:31Z

Concept ACK. What's the point of MacOSIdleInhibitor though? In #5804, CIdleInhibitor was introduced as a generic object that could used to be hold any/all platform-specific inhibitors. Here, MacOSIdleInhibitor seems to just be an extra level of indirection.

krab · 2018-04-13T03:11:42Z

Well windows and linux can't lower cpu performance per application, on windows user can only change system-wide power plans. This thing pretty much only macos specific, MacOSIdleInhibitor change reverted anyways.

fanquake · 2018-04-19T07:45:53Z

src/qt/bitcoin.cpp

@@ -259,6 +263,21 @@ public Q_SLOTS:
    void startThread();
 };

+#if defined(Q_OS_MACOS)
+// A RAII wrapper around calls to forbid and re-allow entering into
+// low-priority states, specifically OSX's app-nap. For the scope of


nit: macOS instead of OSX

src/qt/macos_appnap.mm

fanquake · 2018-04-19T07:48:05Z

src/qt/macos_appnap.h

@@ -0,0 +1,19 @@
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-2018 The Bitcoin Core developers


You can just use:

// Copyright (c) 2018 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php.

as the copyright header here

theuni · 2018-04-19T15:24:59Z

CIdleInhibitor is still there, maybe an issue with the rebase? BitcoinCore::BitcoinCore can just create a CAppNapInhibitor.

fanquake · 2018-05-29T13:53:38Z

@krab could you now squash your commits.

DrahtBot · 2018-10-20T11:07:22Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#14137 (gui: Add Windows taskbar progress by ken2812221)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Sjors · 2018-10-23T03:41:14Z

tACK 2a14df1 on macOS 10.14 (in July I tested on 10.13)

I previously wrote:

it would be nice to disable app nap only during sync,

This works now. With the app in the background, it enters app nap after a minute or so. If however I rewind the chain a bit e.g. with invalidateblock & reconsiderblock and wait for resync, it won't enter app nap.

It does enter app nap during expensive RPC operations (invalidateblock grinds to a halt), but let's improve that some other time. Conversely, we could also make QT (or a future headless node) conserve energy better when the user is on battery.

I also wrote:

The code looks pretty complicated, which generally suggests that Apple doesn't like this.

The current solution looks much cleaner. E.g. it got rid of the undocumented plist option, and instead uses the documented NSActivityOptions. Some relevant Apple docs:

For future reference, I suggest renaming the PR to "macOS: disable AppNap during sync"

fanquake · 2018-10-31T07:14:41Z

Related upstream ticket QTBUG-43861. If we're lucky, one day Qt might expose an API for this functionality, so we don't have to ship the Objective-C ourselves.

Signed-off-by: Alexey Ivanov <alexey.ivanes@gmail.com>

Sjors · 2018-11-02T17:43:54Z

utACK 1e0f3c4 (rebased)

1e0f3c4 macOS: disable AppNap during sync (Alexey Ivanov) Pull request description: Code based on pull/5804. Tested only on macOS 10.13.3 and should support 10.9+. What macOS versions bitcoin core currently supports? Tree-SHA512: 85809b8d8d8a05169437b4268988da0b7372c29c6da3223ebdc106dc16dcb6d3caa5c52ace3591467005b50a63fd8b2ab1cb071cb4f450032932df25d5063315

laanwj · 2018-11-10T09:40:34Z

utACK 1e0f3c4

1e0f3c4 macOS: disable AppNap during sync (Alexey Ivanov) Pull request description: Code based on pull/5804. Tested only on macOS 10.13.3 and should support 10.9+. What macOS versions bitcoin core currently supports? Tree-SHA512: 85809b8d8d8a05169437b4268988da0b7372c29c6da3223ebdc106dc16dcb6d3caa5c52ace3591467005b50a63fd8b2ab1cb071cb4f450032932df25d5063315

#3024) * Merge bitcoin#12783: macOS: disable AppNap during sync 1e0f3c4 macOS: disable AppNap during sync (Alexey Ivanov) Pull request description: Code based on pull/5804. Tested only on macOS 10.13.3 and should support 10.9+. What macOS versions bitcoin core currently supports? Tree-SHA512: 85809b8d8d8a05169437b4268988da0b7372c29c6da3223ebdc106dc16dcb6d3caa5c52ace3591467005b50a63fd8b2ab1cb071cb4f450032932df25d5063315 * Refactor * Drop `#include <memory>` from `src/qt/bitcoingui.h` Was included by mistake.

dashpay#3024) * Merge bitcoin#12783: macOS: disable AppNap during sync 1e0f3c4 macOS: disable AppNap during sync (Alexey Ivanov) Pull request description: Code based on pull/5804. Tested only on macOS 10.13.3 and should support 10.9+. What macOS versions bitcoin core currently supports? Tree-SHA512: 85809b8d8d8a05169437b4268988da0b7372c29c6da3223ebdc106dc16dcb6d3caa5c52ace3591467005b50a63fd8b2ab1cb071cb4f450032932df25d5063315 * Refactor * Drop `#include <memory>` from `src/qt/bitcoingui.h` Was included by mistake.

* commit '7d8eab2641023c78a72ccd6efc99fc35fd030a46': (32 commits) Add 0.14.0.3 change log to release-notes.md (dashpay#3055) Update release-notes.md for 0.14.0.3 (dashpay#3054) Bump version to 0.14.0.3 and copy release notes (dashpay#3053) Re-verify invalid IS sigs when the active quorum set rotated (dashpay#3052) Remove recovered sigs from the LLMQ db when corresponding IS locks get confirmed (dashpay#3048) Add "instantsendlocks" to getmempoolinfo RPC (dashpay#3047) Use fEnablePrivateSend instead of fPrivateSendRunning Show number of InstantSend locks in Debug Console (dashpay#2919) Optimize on-disk deterministic masternode storage to reduce size of evodb (dashpay#3017) Add "isValidMember" and "memberIndex" to "quorum memberof" and allow to specify quorum scan count (dashpay#3009) Implement "quorum memberof" (dashpay#3004) Bail out properly on Evo DB consistency check failures in ConnectBlock/DisconnectBlock (dashpay#3044) Do not count 0-fee txes for fee estimation (dashpay#3037) Fix broken link in PrivateSend info dialog (dashpay#3031) Merge pull request dashpay#3028 from PastaPastaPasta/backport-12588 Add Dash Core Group codesign certificate (dashpay#3027) Fix osslsigncode compile issue in gitian-build (dashpay#3026) Backport bitcoin#12783: macOS: disable AppNap during sync (and mixing) (dashpay#3024) Remove support for InstantSend locked gobject collaterals (dashpay#3019) [v0.14.0.x] Update release notes for 0.14.0.2 (dashpay#3012) ... # Conflicts: # .gitignore # .travis.yml # configure.ac # doc/man/biblepay-cli.1 # doc/man/biblepay-qt.1 # doc/man/biblepay-tx.1 # doc/man/biblepayd.1 # doc/release-notes.md # src/clientversion.h # src/qt/utilitydialog.cpp

dashpay#3024) * Merge bitcoin#12783: macOS: disable AppNap during sync 1e0f3c4 macOS: disable AppNap during sync (Alexey Ivanov) Pull request description: Code based on pull/5804. Tested only on macOS 10.13.3 and should support 10.9+. What macOS versions bitcoin core currently supports? Tree-SHA512: 85809b8d8d8a05169437b4268988da0b7372c29c6da3223ebdc106dc16dcb6d3caa5c52ace3591467005b50a63fd8b2ab1cb071cb4f450032932df25d5063315 * Refactor * Drop `#include <memory>` from `src/qt/bitcoingui.h` Was included by mistake.

…itialBlockDownload() Introduced in bitcoin#12783 / 1e0f3c4

Summary: Backport of core [[bitcoin/bitcoin#12783 | PR12783]]. Test Plan: ninja all check On OSX: ninja all check check-functional Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Subscribers: PiRK Differential Revision: https://reviews.bitcoinabc.org/D8334

fanquake added the macOS label Mar 26, 2018

jonasschnelli reviewed Mar 26, 2018

View reviewed changes

fanquake requested a review from theuni March 27, 2018 03:49

krab force-pushed the macos-disable-appnap branch from 87ff82f to d143357 Compare March 31, 2018 18:03

theuni reviewed Apr 9, 2018

View reviewed changes

krab force-pushed the macos-disable-appnap branch from ec0c230 to e953dbf Compare April 14, 2018 19:46

fanquake reviewed Apr 19, 2018

View reviewed changes

src/qt/macos_appnap.mm Show resolved Hide resolved

fanquake reviewed Apr 19, 2018

View reviewed changes

fanquake mentioned this pull request Apr 20, 2018

macOS: client v0.15.1 still go in App Nap mode #11896

Closed

krab closed this May 29, 2018

krab force-pushed the macos-disable-appnap branch from 7e84a3e to e76acf3 Compare May 29, 2018 17:00

DrahtBot mentioned this pull request Oct 20, 2018

gui: Add Windows taskbar progress #14137

Closed

krab changed the title ~~macOS: Disable AppNap~~ macOS: disable AppNap during sync Oct 23, 2018

This was referenced Oct 28, 2018

gui: explicitly disable "Dark Mode" appearance on macOS #14593

Merged

qt: Cleanup MacDockIconHandler class #14597

Merged

gui: Add GUIUtil::bringToFront #14123

Merged

DrahtBot added the Needs rebase label Nov 1, 2018

macOS: disable AppNap during sync

1e0f3c4

Signed-off-by: Alexey Ivanov <alexey.ivanes@gmail.com>

krab force-pushed the macos-disable-appnap branch from 2a14df1 to 1e0f3c4 Compare November 1, 2018 15:22

DrahtBot removed the Needs rebase label Nov 1, 2018

laanwj merged commit 1e0f3c4 into bitcoin:master Nov 10, 2018

Sjors mentioned this pull request May 30, 2019

gui: add tip for macOS users to disable auto suspend when display is off while plugged in #16125

Closed

UdjinM6 mentioned this pull request Jul 11, 2019

Backport #12783: macOS: disable AppNap during sync (and mixing) dashpay/dash#3024

Merged

jonasschnelli added a commit to jonasschnelli/bitcoin that referenced this pull request May 18, 2020

relax GUI locks: avoid unnecesarry calls to ::ChainstateActive().IsIn…

feecf49

…itialBlockDownload() Introduced in bitcoin#12783 / 1e0f3c4

jonasschnelli mentioned this pull request May 18, 2020

relax GUI locks: avoid unnecesarry calls to ::ChainstateActive().IsInitialBlockDownload #19007

Closed

jonasschnelli added a commit to jonasschnelli/bitcoin that referenced this pull request May 18, 2020

relax GUI locks: avoid unnecesarry calls to ::ChainstateActive().IsIn…

ec2c94f

…itialBlockDownload() Introduced in bitcoin#12783 / 1e0f3c4

i-to-r mentioned this pull request Jun 7, 2021

[macOS] Fix 10 sec delay between auto-syncs (Nagle’s Algorithm + Delayed ACK, App Nap) aither64/haveclip-core#1

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

macOS: disable AppNap during sync #12783

macOS: disable AppNap during sync #12783

krab commented Mar 26, 2018

Empact commented Mar 26, 2018

jonasschnelli Mar 26, 2018

jonasschnelli Mar 26, 2018

jonasschnelli Mar 26, 2018 •

edited

jonasschnelli commented Mar 26, 2018

Sjors commented Mar 27, 2018

jonasschnelli commented Mar 27, 2018

Sjors commented Mar 27, 2018

laanwj commented Mar 27, 2018

theuni commented Mar 28, 2018

Sjors commented Mar 29, 2018

krab commented Mar 31, 2018

krab commented Mar 31, 2018

laanwj commented Apr 7, 2018

theuni Apr 9, 2018

krab Apr 9, 2018

fanquake commented Apr 12, 2018

theuni commented Apr 12, 2018

krab commented Apr 13, 2018

fanquake Apr 19, 2018

fanquake Apr 19, 2018

theuni commented Apr 19, 2018

fanquake commented May 29, 2018

DrahtBot commented Oct 20, 2018 •

edited

Sjors commented Oct 23, 2018

fanquake commented Oct 31, 2018

Sjors commented Nov 2, 2018

laanwj commented Nov 10, 2018

		@@ -0,0 +1,19 @@
		// Copyright (c) 2009-2010 Satoshi Nakamoto
		// Copyright (c) 2009-2018 The Bitcoin Core developers

macOS: disable AppNap during sync #12783

macOS: disable AppNap during sync #12783

Conversation

krab commented Mar 26, 2018

Empact commented Mar 26, 2018

jonasschnelli Mar 26, 2018

Choose a reason for hiding this comment

jonasschnelli Mar 26, 2018

Choose a reason for hiding this comment

jonasschnelli Mar 26, 2018 • edited

Choose a reason for hiding this comment

jonasschnelli commented Mar 26, 2018

Sjors commented Mar 27, 2018

jonasschnelli commented Mar 27, 2018

Sjors commented Mar 27, 2018

laanwj commented Mar 27, 2018

theuni commented Mar 28, 2018

Sjors commented Mar 29, 2018

krab commented Mar 31, 2018

krab commented Mar 31, 2018

laanwj commented Apr 7, 2018

theuni Apr 9, 2018

Choose a reason for hiding this comment

krab Apr 9, 2018

Choose a reason for hiding this comment

fanquake commented Apr 12, 2018

theuni commented Apr 12, 2018

krab commented Apr 13, 2018

fanquake Apr 19, 2018

Choose a reason for hiding this comment

fanquake Apr 19, 2018

Choose a reason for hiding this comment

theuni commented Apr 19, 2018

fanquake commented May 29, 2018

DrahtBot commented Oct 20, 2018 • edited

Conflicts

Sjors commented Oct 23, 2018

fanquake commented Oct 31, 2018

Sjors commented Nov 2, 2018

laanwj commented Nov 10, 2018

jonasschnelli Mar 26, 2018 •

edited

DrahtBot commented Oct 20, 2018 •

edited