Disable gevent on non-Windows. #398

Closed
fiatflux wants to merge 1 commits from pow_deadlock_fix into master
fiatflux commented 2013-08-11 17:40:40 +02:00 (Migrated from github.com)

Fixes #312 and #396. Makes #313 redundant.

Fixes #312 and #396. Makes #313 redundant.
Atheros1 commented 2013-08-12 03:52:43 +02:00 (Migrated from github.com)

What does gevent do for Windows users? From what I remember an OSX user wanted it and added it.

What does gevent do for Windows users? From what I remember an OSX user wanted it and added it.
fiatflux commented 2013-08-12 10:10:46 +02:00 (Migrated from github.com)

I gather its primary advantage is nonblocking IO. That is also its primary disadvantage when doing multiprocessing without care, since a lot of stuff in Python doesn't expect things like the socket module to by asynchronous. Based on what I dug up on the internet, its a problem especially common on POSIX-compliant machines. The problems seem common enough (see cited issues above, and a couple threads on forum) that I think we should immediately disable gevent at least for Linux and therefore probably for OSX too. Another option is to just revert 0aa7efa.

I gather its primary advantage is nonblocking IO. That is also its primary disadvantage when doing multiprocessing without care, since a lot of stuff in Python doesn't expect things like the socket module to by asynchronous. Based on what I dug up on the internet, its a problem especially common on POSIX-compliant machines. The problems seem common enough (see cited issues above, and a couple threads on forum) that I think we should immediately disable gevent at least for Linux and therefore probably for OSX too. Another option is to just revert 0aa7efa.
grant-olson commented 2013-08-12 17:02:46 +02:00 (Migrated from github.com)

FWIW, I just confirmed that I've been running just fine on OSX without gevent installed.

master-blaster:PyBitmessage grant$ python2.7 src/bitmessagemain.py 
Not using the gevent module as it was not found. No need to worry.

So we don't really need it for OSX.

FWIW, I just confirmed that I've been running just fine on OSX without gevent installed. ``` master-blaster:PyBitmessage grant$ python2.7 src/bitmessagemain.py Not using the gevent module as it was not found. No need to worry. ``` So we don't really need it for OSX.
fiatflux commented 2013-08-12 17:44:47 +02:00 (Migrated from github.com)

Aye, it's not a requirement. The idea is to improve performance by using green threads on various components like the socket module. But when that attempt to improve performance vitiates PoW multiprocessing and, according to #396 makes some sockets fail in yet-to-debug circumstances, I'm not wont to consider it an improvement.

BTW, the reason I noted that we could revert 0aa7efa and not the rest of the gevent stuff is because I have yet to encounter or hear of gevent problems on the Qt side of things. (That said, I am not convinced the GUI is even getting the gevent patches if socket is imported first in bitmessagemain.py without the patches... but I haven't looked into this.)

Aye, it's not a requirement. The idea is to improve performance by using green threads on various components like the socket module. But when that attempt to improve performance vitiates PoW multiprocessing and, according to #396 makes some sockets fail in yet-to-debug circumstances, I'm not wont to consider it an improvement. BTW, the reason I noted that we could revert 0aa7efa and not the rest of the gevent stuff is because I have yet to encounter or hear of gevent problems on the Qt side of things. (That said, I am not convinced the GUI is even getting the gevent patches if socket is imported first in bitmessagemain.py without the patches... but I haven't looked into this.)
Atheros1 commented 2013-08-13 01:55:31 +02:00 (Migrated from github.com)

There was a pull request from someone a while back which added gevent and added a background worker thread which doesn't do anything currently. I will manually take it all out (and enjoy it).

There was a pull request from someone a while back which added gevent and added a background worker thread which doesn't do anything currently. I will manually take it all out (and enjoy it).
fiatflux commented 2013-08-13 12:35:23 +02:00 (Migrated from github.com)

and enjoy it

8-)

> and enjoy it 8-)
This repo is archived. You cannot comment on pull requests.
No description provided.