Unify PoW implementation context #59

furuame · 2018-08-14T16:36:48Z

implcontext.h provides a set of API for PoW Implementations for the new interface #57 :

registerImplContext(ImplContext *impl_ctx);
- Register the PoW implementation into the linked list and the dcurl_entry would iterate the linked list to find the available implementation.
initializeImplContext(ImplContext *impl_ctx);
- Initialize the context of PoW implmentation, which is provided by PoW implementation.
destroyImplContext(ImplContext *impl_ctx);
- Free the allocated memory by PoW implementation, which is also provided by PoW implementation.
enterImplContext(ImplContext *impl_ctx);
- Multi-thread management, which enables the thread get the PoW context
exitImplContext(ImplContext *impl_ctx);
- Multi-thread management, which exits the critical section and let the other thread in
getPoWContext(ImplContext *impl_ctx, int8_t *trytes, int mwm);
- Get the PoW context, which is provided by PoW implementation
doThePoW(ImplContext *impl_ctx, void *pow_ctx);
- Do the PoW
freePoWContext(ImplContext *impl_ctx, void *pow_ctx);
- Free the PoW context
getPoWResult(ImplContext *impl_ctx, void *pow_ctx);
- Get the calculated trytes from PoW context

In this pull request, pure C, SSE, AVX and OpenCL implementations are updated for this new interface and pass the tests.

cwei@creeper ~/w/dcurl> make BUILD_GPU=1 BUILD_DEBUG=1 check
*** Validating build/test-trinary ***
	[ Verified ]
*** Validating build/test-curl ***
	[ Verified ]
*** Validating build/test-dcurl ***
	[ Verified ]
*** Validating build/test-multi-pow ***
	[ Verified ]
*** Validating build/test-pow_sse ***
	[ Verified ]
*** Validating build/test-pow_cl ***
	[ Verified ]

The new interface also enables multi-gpu support #24.

+-----------------------------------------------------------------------------+
| NVIDIA-SMI 390.77                 Driver Version: 390.77                    |
|-------------------------------+----------------------+----------------------+
| GPU  Name        Persistence-M| Bus-Id        Disp.A | Volatile Uncorr. ECC |
| Fan  Temp  Perf  Pwr:Usage/Cap|         Memory-Usage | GPU-Util  Compute M. |
|===============================+======================+======================|
|   0  GeForce GTX 1070    Off  | 00000000:01:00.0 Off |                  N/A |
| 11%   57C    P2   103W / 151W |    445MiB /  8117MiB |     95%      Default |
+-------------------------------+----------------------+----------------------+
|   1  GeForce GTX 1070    Off  | 00000000:02:00.0 Off |                  N/A |
|  8%   55C    P2    79W / 151W |    445MiB /  8119MiB |    100%      Default |
+-------------------------------+----------------------+----------------------+

+-----------------------------------------------------------------------------+
| Processes:                                                       GPU Memory |
|  GPU       PID   Type   Process name                             Usage      |
|=============================================================================|
|    0     22720      C   /usr/bin/python3                             435MiB |
|    1     22720      C   /usr/bin/python3                             435MiB |
+-----------------------------------------------------------------------------+

jserv · 2018-08-15T04:56:41Z

Can you rebase?

The current version only support SSE implementation and the multi-threaded scenario has not been tested yet. And the other implementations in the dcurl_entry are removed in the developing period. The dcurl_entry will be more readable and easy to add new implmentation in the future. Testing steps: * make * ./build/test-dcurl

A more general way to iterate the registered PoW implementations

The use of the semaphore can avoid unnecssary spinlock, which results in about 5% performance loss.

Since the dcurl would be compiled for gpu or non-gpu, multi-pow test case for gpu specifically is unnecessary.

furuame · 2018-08-15T11:48:19Z

done.

jserv · 2018-08-16T10:15:04Z

src/clcontext.c

@@ -9,26 +9,14 @@
 #include <stdio.h>
 #include "pearl.cl.h"

+#define HASH_LENGTH 243               // trits


Why don't you refer these macro in src/constants.h?

jserv · 2018-08-16T10:16:27Z

src/clcontext.c

 {
-    *ctx = (CLContext *) malloc(sizeof(CLContext));
+    ctx->kernel_info.buffer_info[0] =


These indexes look a bit ugly. Can you rewrite by mnemonic enum?

jserv · 2018-08-16T10:17:01Z

src/clcontext.c

+        free(devices);
+    }
+    free(platform);
+exit:


Don't use exit, which conflicts with system call.

jserv · 2018-08-16T10:17:34Z

src/dcurl.c

-/* number of task that CPU can execute concurrently */
-static int MAX_CPU_THREAD = -1;
+/* check whether dcurl is initialized */
+static int isInitialized = 0;


Use <stdbool.h> instead of numbers.

jserv · 2018-08-16T10:19:22Z

src/pow_sse.h

+    /* Management of Multi-thread */
+    int indexOfContext;
+    /* Arguments of PoW */
+    int8_t input_trytes[2673]; /* 2673 */


Can we avoid the exposure of constant 2673 here?

jserv · 2018-08-16T10:20:55Z

src/pow_sse.c

@@ -169,13 +166,13 @@ static int64_t loop128(__m128i *lmid,
                       __m128i *hmid,
                       int m,
                       int8_t *nonce,
-                       int id)
+                       int *stopSignal)


I am not sure if stopSignal is an effective name.

It is used to signal other threads stop.

It might be ambiguous to mix stop and signal especially for people thinking it as SIGSTOP.

Okay, I will fix it.

jserv · 2018-08-16T10:21:31Z

src/pow_cl.h

+    /* Management of Multi-thread */
+    int indexOfContext;
+    /* Arguments of PoW */
+    int8_t input_trytes[2673]; /* 2673 */


Ditto. All constants should be referred in unified header.

furuame requested a review from jserv August 14, 2018 16:36

furuame added 15 commits August 15, 2018 19:39

The linked list for ImplContext is introduced

15dabf7

A more general way to iterate the registered PoW implementations

Reduce the number of working thread when leaving

c341923

Unit test for pow_sse is updated for new interface

9f3de39

Fix the improper initialization of SSE context

bb845d4

Update the multi-thread pow test for new interface

5cae697

The use of semaphore boosts the performance

9e49f23

The use of the semaphore can avoid unnecssary spinlock, which results in about 5% performance loss.

Update the C implementation for new interface

c7efd7d

Update the AVX implementation for new interface

b5f29c0

Let dcurl configure every GPU when initializing

c02b45e

Update the OpenCL implementation for new interface

32158a9

Add comment

f12c41b

Remove duplicate multi-pow test case

14ff0af

Since the dcurl would be compiled for gpu or non-gpu, multi-pow test case for gpu specifically is unnecessary.

Free the allocated memory and handle the exception

12c0702

Make the test case more robust

eb618db

furuame force-pushed the refactor-interface-design branch from da79d4a to eb618db Compare August 15, 2018 11:45

DLTcollab deleted a comment from furuame Aug 16, 2018

jserv reviewed Aug 16, 2018

View reviewed changes

src/clcontext.c Outdated

free(devices);

}

free(platform);

exit:

Copy link

Member

jserv Aug 16, 2018

Choose a reason for hiding this comment

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

Don't use exit, which conflicts with system call.

jserv reviewed Aug 16, 2018

View reviewed changes

jserv changed the title ~~A more general dcurl_entry is implemented~~ Unify PoW implementation context Aug 16, 2018

jserv requested a review from ajblane August 16, 2018 10:23

Let constants be referred in the unified header

ddd977f

furuame added 4 commits August 16, 2018 21:45

Replace numbers with mnemonic enum

364500b

Free the memory used to query OpenCL platforms

365c712

Replace the number (0/1) with boolean type

04c73d5

Handle the uninitialized situation

ffd47c7

furuame force-pushed the refactor-interface-design branch from 013c12b to ffd47c7 Compare August 16, 2018 13:45

Rename some varaible to avoid ambiguity

dd26cba

jserv merged commit 4ffade1 into dev Aug 16, 2018

This was referenced Aug 17, 2018

Refactor PoW implementation context #57

Closed

Multi-GPU support #35

Closed

ajblane mentioned this pull request Aug 19, 2018

Fine-grained memory operation with dcurl_entry #56

Closed

furuame deleted the refactor-interface-design branch August 19, 2018 07:04

ajblane added a commit to ajblane/dcurl that referenced this pull request Aug 20, 2018

Modify it to the new interface DLTcollab#59

119e67d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify PoW implementation context #59

Unify PoW implementation context #59

furuame commented Aug 14, 2018

jserv commented Aug 15, 2018

furuame commented Aug 15, 2018

jserv Aug 16, 2018

jserv Aug 16, 2018

jserv Aug 16, 2018

jserv Aug 16, 2018

jserv Aug 16, 2018

jserv Aug 16, 2018

furuame Aug 16, 2018

jserv Aug 16, 2018

furuame Aug 16, 2018

jserv Aug 16, 2018

Unify PoW implementation context #59

Unify PoW implementation context #59

Conversation

furuame commented Aug 14, 2018

jserv commented Aug 15, 2018

furuame commented Aug 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment