SmallBank benchmark implementation #45

supl · 2021-08-03T05:55:28Z

This PR implements the smallbank applications as benchmark tools.

supl · 2021-08-04T05:24:04Z

@feeblefakie @jnmt
Please review this PR.

I copied related contracts from the caliper project, and also have a default client.properties.json file.
I don't use the yargs package for parsing CLI arguments to simplify package.json.

Please kindly advise.
Thank you :)

feeblefakie

Overall looking good. Thank you!
Let me start off with some questions.

feeblefakie · 2021-08-04T09:54:48Z

benchmark/smallbank-bench

+  const [
+    properties,
+    numberOfAccount,
+    numberOfThread,


I understand this is named for consistency, but I think it should be renamed to concurrency or something since Javascript is single-threaded?

Sorry I forgot, maybe adding a readme that details the execution order of the 3 programs would be helpful.

Thank you.
Will do.

feeblefakie · 2021-08-04T10:11:03Z

benchmark/smallbank-bench

+
+  const start = Date.now();
+  let from = start;
+  for (let i = 0; i < numberOfThread; i++) {


Is it using a single CPU core, right?
Java bench is using multiple cores using threads, so I'm wondering how we can compare them.

@feeblefakie

I think the async functions (the one that executes contracts) can leverage multiple CPU cores because they are non-blocking functions.
I am not exactly sure the details though.
Let me check this.

There is another way.
Node.js provides a concurrent practice called worker thread.
(It seems to me like a child process in runtime)
This for sure is leveraging multiple CPU cores.

If the async function practice doesn't really use multiple CPU cores,
then I will try to use worker thread to do it.

feeblefakie · 2021-08-05T00:17:09Z

@supl Are we supposed to be able to run it just by running ./smallbank-bench?
It doesn't work for me, so I'm wondering if anything else is needed.
Sorry I'm a very beginner of node.js.

$ ./smallbank-bench
node:internal/modules/cjs/loader:927
  throw err;
  ^

Error: Cannot find module '@scalar-labs/scalardl-javascript-sdk-base'
Require stack:
- /Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/scalardl-node-client-sdk.js
- /Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/benchmark/smallbank-bench
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:924:15)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Module.require (node:internal/modules/cjs/loader:996:19)
    at require (node:internal/modules/cjs/helpers:92:18)
    at Object.<anonymous> (/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/scalardl-node-client-sdk.js:10:5)
    at Module._compile (node:internal/modules/cjs/loader:1092:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1121:10)
    at Module.load (node:internal/modules/cjs/loader:972:32)
    at Function.Module._load (node:internal/modules/cjs/loader:813:14)
    at Module.require (node:internal/modules/cjs/loader:996:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/scalardl-node-client-sdk.js',
    '/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/benchmark/smallbank-bench'
  ]
}

Torch3333 · 2021-08-05T08:01:11Z

@supl Are we supposed to be able to run it just by running ./smallbank-bench?
It doesn't work for me, so I'm wondering if anything else is needed.
Sorry I'm a very beginner of node.js.

$ ./smallbank-bench
node:internal/modules/cjs/loader:927
  throw err;
  ^

Error: Cannot find module '@scalar-labs/scalardl-javascript-sdk-base'
Require stack:
- /Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/scalardl-node-client-sdk.js
- /Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/benchmark/smallbank-bench
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:924:15)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Module.require (node:internal/modules/cjs/loader:996:19)
    at require (node:internal/modules/cjs/helpers:92:18)
    at Object.<anonymous> (/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/scalardl-node-client-sdk.js:10:5)
    at Module._compile (node:internal/modules/cjs/loader:1092:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1121:10)
    at Module.load (node:internal/modules/cjs/loader:972:32)
    at Function.Module._load (node:internal/modules/cjs/loader:813:14)
    at Module.require (node:internal/modules/cjs/loader:996:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/scalardl-node-client-sdk.js',
    '/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/benchmark/smallbank-bench'
  ]
}

I ran into the same issue. I think you need to run npm install in /Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/

Torch3333

LGTM ! Thank you
I just left a minor comment.

I tested locally both with the auditor enabled and disabled and did not run into problem 🙂

Torch3333 · 2021-08-05T08:02:51Z

benchmark/smallbank-bench

+  console.log(
+      '\t--properties PROPERTIES_FILE, --config PROPERTIES_FILE\n' +
+      '\t\tA configuration file in properties format.\n' +
+      '\t--num-accounts NUM_ACCOUNTS\n' +
+      '\t\tThe number of target accounts.\n' +
+      '\t--num-threads NUM_THREADS\n' +
+      '\t\tThe number of threads to run.\n' +
+      '\t--duration DURATION\n' +
+      '\t\tThe duration of benchmark in seconds\n' +
+      '\t--ramp-up-time RAMP_UP_TIME\n' +
+      '\t\tThe ramp up time in seconds.\n' +
+      '\t--operation OPERATION\n' +
+      '\t\ttransact_savings, ' +
+      'deposit_checking, send_payment, write_check, amalgamate',


Should the default value for each parameter be mentioned in the usage ?

Torch3333 · 2021-08-05T08:03:40Z

benchmark/smallbank-loader

+function usage() {
+  const path = require('path');
+  console.log(
+      'Usage: ' +
+      `${path.basename(process.argv[1])} ` +
+      '[--properties PROPERTIES_FILE, --config PROPERTIES_FILE] ' +
+      '[--num-accounts NUM_ACCOUNTS] ' +
+      '[--num-threads NUM_THREADS] ' +
+      '[-h, --help]',
+  );


Same here. Should the default value for each parameter be mentioned in the usage ?

Torch3333

Sorry I forgot, maybe adding a readme that details the execution order of the 3 programs would be helpful.

jnmt

Overall looking good. Nice work! I was able to run the benchmark. Just a single comment on the discussion here (#45 (comment)).
JS async function does not run with multiple threads/CPUs. In the case of Caliper, child_process is used. Just for your information. Thanks.
https://github.com/scalar-labs/caliper/blob/ee6f3f2c5d69ad944ca753b6d5bb567849731285/packages/caliper-core/lib/master/orchestrators/worker-orchestrator.js#L548

supl · 2021-08-11T03:24:14Z

@jnmt
Jun, thanks for your recommendation.
It's really helpful!
I was thinking to use worker_threads to revise the application.
Unfortunately, the gRPC library seems not to support working with worker_threads.
I end up doing this by child_process as your advice.
Thank you so much :)

@feeblefakie
I revised the smallbank-loader.
Since it doesn't use threading but just async functions, so I changed the option to num-concurrencies for a clear idea.
The smallbank-bench uses child_process to achieve parallel processing, and I named the option num-processes.

Please review again.
Thank you :)

jnmt

LGTM! Multi processes work well. It might not be a high priority but it would be happy if we could run loader with multi cores as well. Thank you.

feeblefakie

Thank you!
Left one suggestion. PTAL when you get a chance.

feeblefakie · 2021-08-11T11:43:15Z

benchmark/smallbank-bench

+        }
+        break;
+
+      case '--num-processes':


Sorry, we should have discussed the way to do it.

I think just using multiple processes is too efficient since a process is heavy so I think we should mix processes and threads (asynchronous multi-tasking) to make it comparable to the Java version.

One way to do this is that we let users specify concurrency and make the tool automatically divide the concurrency into multiple processes and "threads".
For example, if 16 concurrency is given on 4-core machine, we can create 4 processes to match with the number cores and create 4 "threads" for each process.

What do you think?

I see.
I didn't think about that.
So, a sub-process will be in charge of a few concurrencies.
Unfortunately, we couldn't use worker_threads to implement it otherwise it might be more similar to Java's implementation.
I will revise smallbank-bench and smallbank-loader (according to Jun's feedback) by this idea.

Sorry, I know we don't have threads. I meant multitasking with async.

Sorry, we should have discussed the way to do it.

I think just using multiple processes is too efficient since a process is heavy so I think we should mix processes and threads (asynchronous multi-tasking) to make it comparable to the Java version.

One way to do this is that we let users specify concurrency and make the tool automatically divide the concurrency into multiple processes and "threads".
For example, if 16 concurrency is given on 4-core machine, we can create 4 processes to match with the number cores and create 4 "threads" for each process.

What do you think?

@feeblefakie
I revised the implementation of smallbank-bench.
First, I rename the option name back to --num-concurrencies.
smallbank-bench now divides the requested concurrencies by the number of CPU,
then it forks subprocesses as the same number as CPU.
Each forked subprocess uses async functions with the number of divided concurrencies to run the benchmark.

I also revised smallbank-loader with the child_process mechanism.
I didn't use fork + async way for smallbank-loader because I think it doesn't matter that smallbank-loader does works too efficiently, so I used an easier way to revise it.

@jnmt
Jun, thank you for your advice.
I used child_process to revise smallbank-loader.
Now it can do jobs more efficiently too.

feeblefakie · 2021-08-13T05:29:48Z

@supl I've played around with it but am facing some issues.
Even after npm install, I am facing the following error.

$ benchmark/smallbank-bench --properties ~/git/scalar/client/client.properties --num-accounts 100 --num-concurrencies 4 --duration 10  --ramp-up-time 1
0 tps
node:internal/modules/cjs/loader:927
  throw err;
  ^

Error: Cannot find module '/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/smallbank-bench-worker'
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:924:15)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12)
    at node:internal/main/run_main_module:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}
node:internal/modules/cjs/loader:927
  throw err;
  ^

Error: Cannot find module '/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/smallbank-bench-worker'
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:924:15)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12)
    at node:internal/main/run_main_module:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}
node:internal/modules/cjs/loader:927
  throw err;
  ^

Error: Cannot find module '/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/smallbank-bench-worker'
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:924:15)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12)
    at node:internal/main/run_main_module:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}
node:internal/modules/cjs/loader:927
  throw err;
  ^

Error: Cannot find module '/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/smallbank-bench-worker'
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:924:15)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12)
    at node:internal/main/run_main_module:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}
0 tps

feeblefakie

Thank you!
I left a suggestion.
I'm also facing some issues running it yet.

feeblefakie · 2021-08-13T05:31:01Z

benchmark/smallbank-bench

+        }
+        break;
+
+      case '--ramp-up-time':


When I specified 0 for this, it still triggers the help. 0 is useful for testing, so it would be great if it is fixed!

No problem.

feeblefakie · 2021-08-13T05:33:57Z

benchmark/README.md

+```
+
+```
+./smallbank-bench -h


Also, it would be great if the help is shown when nothing is specified.

No problem.

supl · 2021-08-13T05:42:20Z

@supl I've played around with it but am facing some issues.
Even after npm install, I am facing the following error.

$ benchmark/smallbank-bench --properties ~/git/scalar/client/client.properties --num-accounts 100 --num-concurrencies 4 --duration 10  --ramp-up-time 1
0 tps
node:internal/modules/cjs/loader:927
  throw err;
  ^

Error: Cannot find module '/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/smallbank-bench-worker'
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:924:15)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12)
    at node:internal/main/run_main_module:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}
node:internal/modules/cjs/loader:927
  throw err;
  ^

Error: Cannot find module '/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/smallbank-bench-worker'
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:924:15)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12)
    at node:internal/main/run_main_module:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}
node:internal/modules/cjs/loader:927
  throw err;
  ^

Error: Cannot find module '/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/smallbank-bench-worker'
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:924:15)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12)
    at node:internal/main/run_main_module:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}
node:internal/modules/cjs/loader:927
  throw err;
  ^

Error: Cannot find module '/Users/hiroyuki/Dropbox/doc/git/scalardl-node-client-sdk/smallbank-bench-worker'
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:924:15)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12)
    at node:internal/main/run_main_module:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}
0 tps

This must be an issue related to the working directory.
Let me fix it.

Thank you for your testing :)

supl · 2021-08-13T06:41:51Z

@feeblefakie
I fixed the working directory issue.
The help is now shown when no argument is given and the ramp-up-time can be 0 now!

feeblefakie · 2021-08-16T09:09:56Z

@supl Thank you for the guidance!

I'll share the preliminary benchmarking results in my local environment (that has 4 CPU cores).
I used the same properties file with the following option for smallbank-bench.

--num-accounts 100 --num-threads 4 --duration 10  --ramp-up-time 1

Java SDK (1st time):

TPS: 56.1
Average-Latency(ms): 64.03743315508021
Error-Counts: 79

Java SDK (2nd time):

TPS: 61.7
Average-Latency(ms): 59.79092382495948
Error-Counts: 78

Node SDK (1st time):

TPS: 53.2
Average-Latency(ms): 73.07142857142857
Error-Counts: 25

Node SDK (2nd time):

TPS: 64.9
Average-Latency(ms): 60.003081664098616
Error-Counts: 37

My local environment runs many other processes so it's not a good place for benchmark but
I think they are consistent enough to go :)

feeblefakie

LGTM. Nice work! Thank you!

I think we can further work on the following things in another PR:

Support optional configurations that are consistent with the Java version
Support PEM files (for only node version)
Update documentation to specify version requirements

supl added 3 commits August 2, 2021 19:28

Add sample client_properties.json

d2fc30b

Add contracts of smallbank

db6bc2c

Add script to register contracts for smallbank

7f05b5c

supl force-pushed the DLT-9604 branch 2 times, most recently from 074d589 to c0d5772 Compare August 3, 2021 06:17

Create smallbank-loader

463dacf

supl force-pushed the DLT-9604 branch from c0d5772 to 463dacf Compare August 3, 2021 07:28

supl added 9 commits August 4, 2021 10:04

Implement smallbank-bench

0bda6f9

Revise

03b5888

Remove --trace-warning runtime option

0ab37c0

client_properties.json -> client.properties.json

5ae9dfb

Proper name

a1eaa52

Revise error message

f85a694

Use atomic

9c1f0b2

Shift 1

ef9b439

BigInt

97f7e51

supl marked this pull request as ready for review August 4, 2021 05:19

supl requested review from feeblefakie and jnmt August 4, 2021 05:20

feeblefakie assigned supl Aug 4, 2021

feeblefakie added the improvement label Aug 4, 2021

feeblefakie reviewed Aug 4, 2021

View reviewed changes

feeblefakie requested a review from Torch3333 August 5, 2021 00:07

Torch3333 approved these changes Aug 5, 2021

View reviewed changes

Torch3333 reviewed Aug 5, 2021

View reviewed changes

supl added 2 commits August 6, 2021 11:46

thread -> concurrency

5e43be8

Revise usage print

c7654a3

jnmt reviewed Aug 7, 2021

View reviewed changes

jnmt approved these changes Aug 11, 2021

View reviewed changes

feeblefakie reviewed Aug 11, 2021

View reviewed changes

Revise

6491897

supl force-pushed the DLT-9604 branch from b956e73 to 6491897 Compare August 12, 2021 06:34

supl added 5 commits August 12, 2021 15:48

Rename

e939227

Revise smallbank-loader

4659def

Executable

c7130c4

Remove unused comment

946f33b

Remove unused pool

55eaecf

feeblefakie reviewed Aug 13, 2021

View reviewed changes

supl added 4 commits August 13, 2021 15:14

Handle working directory

d7ff119

Bugfix

308a15c

properties is mandatory without any default value

63a399c

Ramp up time can be 0

6cb6467

supl added 6 commits August 15, 2021 16:33

Use properties-parser package

2585851

Add util to parse Scalar DL client.properties

511d618

Add client.properties

4a61d2b

Use client.properties

9c7d88a

Revise README.md

44d2987

Remove client.properties.json

ce605ad

feeblefakie approved these changes Aug 16, 2021

View reviewed changes

feeblefakie merged commit a05e37e into master Aug 16, 2021

feeblefakie deleted the DLT-9604 branch August 16, 2021 09:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SmallBank benchmark implementation #45

SmallBank benchmark implementation #45

supl commented Aug 3, 2021 •

edited

Loading

supl commented Aug 4, 2021

feeblefakie left a comment

feeblefakie Aug 4, 2021 •

edited

Loading

supl Aug 5, 2021

feeblefakie Aug 4, 2021

supl Aug 5, 2021

feeblefakie commented Aug 5, 2021 •

edited

Loading

Torch3333 commented Aug 5, 2021 •

edited

Loading

Torch3333 left a comment

Torch3333 Aug 5, 2021

Torch3333 Aug 5, 2021

Torch3333 left a comment •

edited

Loading

jnmt left a comment

supl commented Aug 11, 2021

jnmt left a comment

feeblefakie left a comment

feeblefakie Aug 11, 2021

supl Aug 12, 2021

feeblefakie Aug 12, 2021

supl Aug 12, 2021 •

edited

Loading

feeblefakie commented Aug 13, 2021

feeblefakie left a comment

feeblefakie Aug 13, 2021

supl Aug 13, 2021

feeblefakie Aug 13, 2021

supl Aug 13, 2021

supl commented Aug 13, 2021

supl commented Aug 13, 2021

feeblefakie commented Aug 16, 2021 •

edited

Loading

feeblefakie left a comment

SmallBank benchmark implementation #45

SmallBank benchmark implementation #45

Conversation

supl commented Aug 3, 2021 • edited Loading

supl commented Aug 4, 2021

feeblefakie left a comment

Choose a reason for hiding this comment

feeblefakie Aug 4, 2021 • edited Loading

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

feeblefakie commented Aug 5, 2021 • edited Loading

Torch3333 commented Aug 5, 2021 • edited Loading

Torch3333 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Torch3333 left a comment • edited Loading

Choose a reason for hiding this comment

jnmt left a comment

Choose a reason for hiding this comment

supl commented Aug 11, 2021

jnmt left a comment

Choose a reason for hiding this comment

feeblefakie left a 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

supl Aug 12, 2021 • edited Loading

Choose a reason for hiding this comment

feeblefakie commented Aug 13, 2021

feeblefakie left a 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

supl commented Aug 13, 2021

supl commented Aug 13, 2021

feeblefakie commented Aug 16, 2021 • edited Loading

feeblefakie left a comment

Choose a reason for hiding this comment

supl commented Aug 3, 2021 •

edited

Loading

feeblefakie Aug 4, 2021 •

edited

Loading

feeblefakie commented Aug 5, 2021 •

edited

Loading

Torch3333 commented Aug 5, 2021 •

edited

Loading

Torch3333 left a comment •

edited

Loading

supl Aug 12, 2021 •

edited

Loading

feeblefakie commented Aug 16, 2021 •

edited

Loading