Skip to content

New POW calculation module #1284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 38 commits into
base: v0.6
Choose a base branch
from
Open

New POW calculation module #1284

wants to merge 38 commits into from

Conversation

Kleshni
Copy link

@Kleshni Kleshni commented Jun 23, 2018

For #1227. It's currently not connected to the PyBitmessage code, I'm going to get it done sooner or later.

It provides the WorkProver thread class which manages tasks and schedules them to solvers. There are 4 possible solvers:

  • Single-threaded DumbSolver in Python. I optimised it so it now works ~2 times faster. In particular, I replaced hashlib.sha512 with SHA512 from OpenSSL. Since you need OpenSSL to assemble a message anyway, I think it's safe to use it in a fallback solver.

  • Multiprocess based ForkingSolver in Python. It works faster too, because it relies on DumbSolver.

  • Multithreaded FastSolver in C. I tried to utilize SIMD, but it gave very little speed up of 7 %, so it still calls OpenSSL.

  • And GPUSolver in OpenCL.

The library was tested on a 4-core AMD64 Linux with integrated GPU and 45-CPU virtual machines with hyperthreading enabled:

  • X86 Linux;
  • AMD64 FreeBSD;
  • AMD64 OpenBSD with the bsd.mp kernel;
  • AMD64 NetBSD;
  • AMD64 Windows 7.

I failed to install Hackintosh on QEMU, so somebody else has to test it there.

@PeterSurda
Copy link
Member

Have you tried frozen mode on Windows?

@Kleshni
Copy link
Author

Kleshni commented Jun 23, 2018

No, but ForkingSolver is disabled in this case complying to the current behaviour. All file paths (to the C library and OpenCL kernel) are relative to the code directory, whose path must be provided by the calling code.

@@ -0,0 +1,42 @@
Please keep this module independent from the outside code, so that it can be reused in other applications.

Choose a reason for hiding this comment

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

In that case why can't we make it a separate package and import it where required ?

Copy link
Author

Choose a reason for hiding this comment

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

It's a Python package with the __init__.py file, and it's intended to be imported like import workprover. It could be moved to a separate repository, but I think, it's easier and safer to keep it in the same file tree.

try:
self.availableSolvers["fast"] = fastsolver.FastSolver(codePath)
except:
pass

Choose a reason for hiding this comment

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

pass is the dangerous statement, please log it or please write a print statement atleast...

Copy link
Author

Choose a reason for hiding this comment

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

self.availableSolvers is visible to the outside code, so it can log or print if fast is missing. It should show a message in GUI status bar like the current code does.

if self.statusUpdated is None:
return

if self.solver is None:

Choose a reason for hiding this comment

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

parallelism = self.solver.parallelism if self.solver else 0

Copy link
Author

Choose a reason for hiding this comment

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

It's less readable.

self.currentTaskID = None

def notifyStatus(self):
if self.statusUpdated is None:

Choose a reason for hiding this comment

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

if not self.statusUpdated:
return

Copy link
Author

Choose a reason for hiding this comment

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

It's less acurate.

self.statusUpdated must be either a function or a None. If it's 0 or an empty string, it would be a programming error and further call would rise an exception making the error noticable.

self.statusUpdated((self.solverName, parallelism, self.speed))

def setSolver(self, name, parallelism):
if name is None and self.solverName is None:

Choose a reason for hiding this comment

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

if not name and not self.solverName

import ctypes

def loadFastSolver(codePath):
if hasattr(sys, "winver"):

Choose a reason for hiding this comment

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

doc type required here


if platform.architecture()[0] == "64bit":
suffix = "-64"

Choose a reason for hiding this comment

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

pep8 validation

self.parallelism = 0

def search(self, initialHash, target, seed, timeout):
found = self.libfastsolver.fastsolver_search(

Choose a reason for hiding this comment

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

documentation required here

@@ -0,0 +1,106 @@
import os

Choose a reason for hiding this comment

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

pep8 validation for the file

def setIdle():
try:
import psutil

Choose a reason for hiding this comment

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

remove un necessary spaces

Copy link
Collaborator

@g1itch g1itch left a comment

Choose a reason for hiding this comment

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

Seems to work for me: two "singleWorker" threads appear when doing PoW. Oh, I realized it's not connected so far :(

@g1itch
Copy link
Collaborator

g1itch commented Jun 23, 2018

@Kleshni workprover.test also contains tabs

@g1itch
Copy link
Collaborator

g1itch commented Jun 23, 2018

Some tests are failing. It seems to be expected for TestGPUSolver, what about the rest?

@Kleshni
Copy link
Author

Kleshni commented Jun 23, 2018

TestSolver is not a test case, it's a base class for other test cases. Now it should be skipped during automatic test discovery.

TestFastSolver fails loading compiled libfastsolver.so for the reason I don't know. A print in the corresponding except could clarify this.

@g1itch
Copy link
Collaborator

g1itch commented Jun 23, 2018

Strange thing:

OSError: /home/travis/build/g1itch/PyBitmessage/src/workprover/fastsolver/libfastsolver.so: undefined symbol: SHA512_Update

It seems to be specific to ubuntu:trusty or Travis-CI.

Nevertheless could you please add pybitmessage.workprover to packages and decorate TestGPUSolver like I did?

@Kleshni
Copy link
Author

Kleshni commented Jun 25, 2018

I have registered in Travis-CI and debugged the issue. It was a very stupid mistake, shared libraries like -lpthread and -lcrypto should be listed after object files in the linker command line.

Travis-CI also gives an OS X environment, so I tested the module there.

@PeterSurda
Copy link
Member

packages/pyinstaller/bitmessagemain.spec needs updating.


try:
import numpy
import pyopencl
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please comment #1283 and maybe change the import order too if you agree?

@@ -54,6 +54,8 @@ else:
a.binaries += [('libeay32.dll', openSSLPath + 'libeay32.dll', 'BINARY'),
(os.path.join('bitmsghash', 'bitmsghash%i.dll' % (arch)), os.path.join(srcPath, 'bitmsghash', 'bitmsghash%i.dll' % (arch)), 'BINARY'),
(os.path.join('bitmsghash', 'bitmsghash.cl'), os.path.join(srcPath, 'bitmsghash', 'bitmsghash.cl'), 'BINARY'),
("workprover/fastsolver/libfastsolver-{}.dll".format(arch), os.path.join(srcPath, "workprover/fastsolver/libfastsolver-{}.dll".format(arch)), "BINARY"),

Choose a reason for hiding this comment

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

PEP8 validation required here.


# This thread, of which there is only one, runs the API.
class singleAPI(threading.Thread, helper_threading.StoppableThread):
def __init__(self):

Choose a reason for hiding this comment

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

please add doc string to the class.

self.initStop()

def stopThread(self):
super(singleAPI, self).stopThread()

Choose a reason for hiding this comment

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

please add doc string to the class

@@ -502,10 +384,85 @@ def main():
mainprogram = Main()
mainprogram.start()

# See "workprover/Readme.md"

import os

Choose a reason for hiding this comment

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

please write all the imports on the starting of the file.


if __name__ == "__main__":
main()
import multiprocessing

Choose a reason for hiding this comment

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

Please place all the imports in the starting the file...

Copy link
Author

Choose a reason for hiding this comment

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

These imports must be conditional.


# Update status values inherited from old versions

self.cur.execute("""UPDATE "sent" SET "status" = 'msgsent' WHERE "status" == 'sentmessage';""")

Choose a reason for hiding this comment

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

pep8 validation here.

@@ -0,0 +1,253 @@
import Queue

Choose a reason for hiding this comment

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

please place all the imports in a alphabetic order.

self.statusUpdated(Status(self.solverName, status, self.speed, len(self.tasks), self.totalDifficulty))

def setSolver(self, name, configuration):
if name is None and self.solverName is None:

Choose a reason for hiding this comment

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

if not name and not self.solverName:
pass

elif name == self.solverName:
self.solver.setConfiguration(configuration)
else:
if self.solver is not None:

Choose a reason for hiding this comment

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

if self.solver:
….

self.solverName = None
self.solver = None

if name is not None:

Choose a reason for hiding this comment

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

if name:
….

@Kleshni
Copy link
Author

Kleshni commented Jul 29, 2018

Added API methods for dealing with raw objects in the inventory, solving #1225 and superseding #1226.

disseminateRawObject

Argument: a hex-encoded object without packet headers, starting from nonce.

Tries to send the object to the network. POW must be already calculated.

Returns an error or the object's inventory hash.

getRawObject

Argument: an inventory hash.

Returns an error or a JSON object with the following fields:

  • hash - the inventory hash;
  • expiryTime - a UNIX timestamp;
  • objectType - an integer;
  • stream;
  • tag - hex-encoded object tag;
  • payload - hex-encoded object without packet headers, starting from nonce.

listRawObjects

Parameters: the desired object type or None, stream number or None, hex-encoded tag or None.

Returns a list of hex-encoded hashes.

queueRawObject

Arguments: TTL in seconds and a hex-encoded object without packet headers, nonce and expiry time, starting from object type.

Queues the object for POW calculation and sending to the network.

Returns a unique handle to track the objects' state.

cancelQueuedRawObject

Argument: a handle returned from the queueRawObject method.

Tries to cancel a previously queued object.

checkQueuedRawObject

Argument: a handle returned from the queueRawObject method.

Returns an array with the first element being the current status of the object:

  • queued;
  • doingwork;
  • failed for invalid objects;
  • sent. In this case the second item of the array is the hex-encoded inventory hash;
  • canceled;
  • notfound for wrong handles and for handles that previously returned failed, sent or canceled.

If a queued object enters the notfound state, it can mean that the daemon was restarted, because the queued objects are not saved to the disk.

@Kleshni
Copy link
Author

Kleshni commented Jul 29, 2018

I deleted the getMessageDataByDestination{Hash,Tag} methods because they were undocumented and didn't work at all, so I concluded that no-one ever used them. They are now replaced by getRawObject and listRawObjects.

The same for disseminatePreEncryptedMsg and disseminatePubkey.

self.solverName = name
self.solver = self.availableSolvers[name]
self.solver.setConfiguration(configuration)
except GPUSolverError:
Copy link
Collaborator

Choose a reason for hiding this comment

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

undefined name 'GPUSolverError'

@g1itch
Copy link
Collaborator

g1itch commented Aug 2, 2018

I got 3 additional threads (workprover.WorkProver instances) which don't stop when I run src/pybitmessage.py -t:
https://travis-ci.org/g1itch/PyBitmessage/builds/411231409

This patch helped:

diff --git a/src/singleworker.py b/src/singleworker.py
index 9fa1391c..e1eeda3f 100644
--- a/src/singleworker.py
+++ b/src/singleworker.py
@@ -270,17 +270,19 @@ def setBestSolver():
 
 setBestSolver()
 
+
 class singleWorker(threading.Thread, helper_threading.StoppableThread):
     name = "singleWorker"
 
     def __init__(self):
-        super(self.__class__, self).__init__()
+        super(self.__class__, self).__init__(name="singleWorker")
 
         self.initStop()
 
     def stopThread(self):
         queues.workerQueue.put(("stopThread", "data"))
 
+        workProver.commandsQueue.put(("shutdown", ))
         super(self.__class__, self).stopThread()
 
     def run(self):

@Kleshni
Copy link
Author

Kleshni commented Sep 17, 2018

Any plans on merging?

@dimyme
Copy link
Contributor

dimyme commented Sep 20, 2018

your POW branch works nicely here ! great job !

@PeterSurda
Copy link
Member

@Kleshni I haven't looked at it yet, but it first needs to pass all the code quality checks and all commits need to be signed (you can squash commits if it helps).

However, others gave good feedback so chances are good.

@PeterSurda
Copy link
Member

Also, please split this into multiple patches. The PoW, the API and the other changes should be separate. You'll have better chances of getting it merged that way too.

@PeterSurda
Copy link
Member

@Kleshni would it be ok if I assigned someone else to the task to help you with cleaning it up? You'd have to give them write access to your repo.

@Kleshni
Copy link
Author

Kleshni commented Apr 24, 2019

OK.

@PeterSurda
Copy link
Member

@Kleshni please give @omkar1117 write access to the POW branch. this way your commits will be preserved and you can still get paid through tip4commit.

@Kleshni
Copy link
Author

Kleshni commented Apr 25, 2019

I have sent him an invite yesterday but forgot to notify you 👀

@omkar1117
Copy link

omkar1117 commented May 8, 2019

@Kleshni could you please resolve the conflicts please.

@PeterSurda
Copy link
Member

@Kleshni Omkar says he doesn't have write access. He could fork it into his own repo, but since this PR is already open I would prefer to continue this way. Can you check and let me know?

@Kleshni
Copy link
Author

Kleshni commented Jun 11, 2019

This is what I see in the repository settings:
Screenshot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants