Skip to content

Conversation

@theuni
Copy link
Member

@theuni theuni commented Feb 18, 2015

Currently for OSX we're disabling app-nap globally via the app's plist. If enabled, app-nap slows down performance badly for long-running activities. See #5041 for more background.

Disabling via the plist has a few draw-backs:

  • The plist setting only has an effect when running from an .app bundle. In practice, that means only when running from a packaged release
  • We don't distribute bitcoind in the bundle and likely never will. So app-nap is always enabled for bitcoind.
  • App-nap can actually be a useful feature. It would be helpful to only disable it at certain times (initial sync for example)

These changes add runtime functionality so that we can turn it on/off at-will. For now, it's enabled for the life-time of the programs, but that can be changed once we decide how to regulate it.

The CIdleInhibitor class is very ugly, but I assume that at some point we'll add similar features for other operating systems. I think this is nicer than osx ifdefs all over the place. I avoided inheritance and virtuals since we're interfacing with objc.

Needs testing on < 10.9 systems.

For now, disable for the life of bitcoind and bitcon-qt to achieve the same
behavior as before. More fine-grained behavior can come after more testing.
Copy link
Member

Choose a reason for hiding this comment

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

Please don't add these kind of one-use utilities in the top-level src directory. As I've suggested before, a an src/support or such directory would make sense.

@laanwj
Copy link
Member

laanwj commented Feb 19, 2015

Sorry to say but I'm not entirely convinced of adding mac-specific code to the core code. In the GUI this is more acceptable (and even there we like to avoid it if possible), but I don't know here. The plist-based solution works, so this reeks a bit of "Don't fix it, it's not broken" to me.

Copy link
Member

Choose a reason for hiding this comment

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

Objective C in the core? :(

@theuni
Copy link
Member Author

theuni commented Feb 19, 2015

Ok. I was under the impression that some of our non-qt deps required objc and frameworks already, but looking at the link-line, that's not the case. So I agree with the no-objc argument.

Closing as not worth it.

@theuni theuni closed this Feb 19, 2015
@jonasschnelli
Copy link
Contributor

Hmm... i kinda liked this. Obj-c in core should be avoided, right. Maybe we find a more modular approach? env_mac.mm? I assume other OS will also come with/have similar low-energy technologies.
Related to #5314 and #5344

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants