Add simple profiling / Basic benchmark & README update

[holgerd77]

Hi @alcuadrado, I thought it would be the most sustainable option to create a PR on your request to send the MPT flamegraph, especially since sending around an HTML flamegraph is a bit tricky.

The PR integrates basic profiling on the library, I also updated the random.ts benchmark a bit, this was actually already quite similar before in comparison to my code I had for profiling.

This PR is of course just one small step to first-introduce this basic profiling here. Everyone feel free to change on everything on subsequent PRs. Once this is merged it should be easily doable to generate a flamegraph with:

npm run profiling

[coveralls]

Coverage remained the same at 95.29% when pulling 6141ed6 on add-profiling into 0e6e1f7 on master.

[alcuadrado]

Hey @holgerd77,

I run the profiler and confirmed that the native version of keccak is being used. I guess your confusion comes form having a frame like this

The hash.js file will be used, regardless of whether the native or js version is being used. But, as you can see, all of the frames on top of the one from keccak are C++ code. In particular, many of them are NAPI calls, like this one:

My understanding is that for the js version to be used one of these things needs to happen:

1. Your OS is not 64bits
2. You are not running on x86
3. Your OS is not linux, mac nor windows.
4. You are running Linux and your os uses a libc other than glibc and musl.
5. Your node version is lower than 10.0.0.

I believe all of them are highly unlikely. If one of them happens, there's not much we can do, as it means that there's no native version available.

Note that there aren't other things that could fail that will lead to the js version being used, as there's no installation process that could go wrong. The prebuilt binaries come bundled within the npm package, and are just used at runtime whenever they match the os & hardware requirements.

[holgerd77]

This is now ready for re-review.

[holgerd77]

@alcuadrado my flamegraph structure looks fundamentally different

I also can't find any napi like references, so I am pretty sure that I am using plain JS. I also renamed the precompiles folder in node_modules for further assurance, no change on execution.

Then I did a fresh installation on MPT (complete new repo and then npm i), same result. To be complete I then also deleted the node cache and did another fresh installation, no change.

My setup here is: MacBookAir 2013 (Intel Core i5), Mojave, Node v12.15.0.

So it might be worth to further investigate here if this is something systemic and breaks the assumptions made on the ethereum-cryptography library and there might be the need for some update or change or if this just some problem on my current local system setup.

It would be good if others could tell from their setups and their perceived keccak installation.

My first-round take away from this is still what we discussed on the call: that it is very much too intransparent what version is used on runtime.

The code on the keccak library here is also very hard to decypher and it is super-tricky to find the entrypoint for the native-vs-JS selection. I had a hard time figuring out what the export in bindings.js does (still not sure if I finally got it).

On a very low level it would already help to rewrite these code parts in a more readable way and/or add some simple comments.

[alcuadrado]

Hey @holgerd77, thanks for investigating this further.

@alcuadrado my flamegraph structure looks fundamentally different

I may be missing something, but I don't see how that portion of the flamegraph is relevant. There's no mention of keccak, ethereum-cryptography, nor ethereumjs-util.

I'm still not sure if you are using the native version.

(PS: I added a way to validate this lower in this comment)

I also can't find any napi like references, so I am pretty sure that I am using plain JS. I also renamed the precompiles folder in node_modules for further assurance, no change on execution.

What do you mean by precompiles folder? That must be something unrelated. I never saw that.

Then I did a fresh installation on MPT (complete new repo and then npm i), same result. To be complete I then also deleted the node cache and did another fresh installation, no change.

Thanks for trying this. I've fallen into that trap so many times 😅

My setup here is: MacBookAir 2013 (Intel Core i5), Mojave, Node v12.15.0.

I wonder if Mojave has anything to do here. Let'a first validate that you are actually using the js version, and if that's the case I'll use a vm to reproduce it.

So it might be worth to further investigate here if this is something systemic and breaks the assumptions made on the ethereum-cryptography library and there might be the need for some update or change or if this just some problem on my current local system setup.

I think this is what is confusing me/us the most. I created that list and I believe it's correct. Sorry for not being clear on that.

These assumptions are not made by ethereum-cryptography, nor even keccack. You could say that node-gyp-build, the loader of the napi module makes it, but that wouldn't be accurate either. As I mentioned on discord, it works something like this:

const napiLoader = require("lib-that-know-how-to-load-napi-things");

try {
  module.exports = napiLoader("./prebuilds");   
} catch (e) {
  module.exports = require("./pure-js.js");
}

so there's no actual place where those assumptions are checked. This works exactly the same as feature detection works on the web.

It would be good if others could tell from their setups and their perceived keccak installation.

My first-round take away from this is still what we discussed on the call: that it is very much too intransparent what version is used on runtime.

I completely agree with this.

As I wrote on discord, we should decide if we want to ask the maintainers of keccak and other native bindings to expose this information at runtime, and also come up with a way of doing that, as printing a message at runtime is not a valid option.

If this info is available, I'll export it in ethereum-cryptography.

The code on the keccak library here is also very hard to decypher and it is super-tricky to find the entrypoint for the native-vs-JS selection. I had a hard time figuring out what the export in bindings.js does (still not sure if I finally got it).

On a very low level it would already help to rewrite these code parts in a more readable way and/or add some simple comments.

To be really honest, I don't think this is the case. The entry point of the library is, as usual, index.js, and it follows the typical pattern used for this in node:

try {
  module.exports = require('./bindings')
} catch (err) {
  module.exports = require('./js')
}

You can manually check which version is being used by adding console.logs right after each module.exports =. I'd add it to both of them, just to make sure I edited the right file.

Also, bindings.js is an internal file, and I don't think it would be nice from us to go to an open source project we consume and barely contribute to, and ask for this kind of changes.

I do find node-gyp-build's code almost impossible to read, but my point is even stronger there, as we have no relationship with them.

Anyway, thanks for digging deeper into this. Can you check if you are actually using the native version by adding those console.logs? Thanks

[holgerd77]

What do you mean by precompiles folder? That must be something unrelated. I never saw that.

Meant prebuilds, sorry for the confusion.

try {
 console.log('1')
  module.exports = require('./bindings')
 console.log('2')
} catch (err) {
 console.log('3')
  module.exports = require('./js')
}

You can manually check which version is being used by adding console.logs right after each module.exports =. I'd add it to both of them, just to make sure I edited the right file.

That was one of the first things I tried some time before I have been writing the post from above. I was so convinced that this went the 1 ->2 way (see modified quote from above). Now I tried again and it went 1 -> 3. 🤨 So seems the native version gets executed. Sorry for all the inconvenience. (I am still a bit unsettled since I was not able yet to find any __napi__ entries in the flamegraph).

What Node version are you actually using? Maybe I'll try run profiling with that version and see if anything changes.

[ryanio]

LGTM!

[alcuadrado]

What do you mean by precompiles folder? That must be something unrelated. I never saw that.

Meant prebuilds, sorry for the confusion.

try {
 console.log('1')
  module.exports = require('./bindings')
 console.log('2')
} catch (err) {
 console.log('3')
  module.exports = require('./js')
}

You can manually check which version is being used by adding console.logs right after each module.exports =. I'd add it to both of them, just to make sure I edited the right file.

That was one of the first things I tried some time before I have been writing the post from above. I was so convinced that this went the 1 ->2 way (see modified quote from above). Now I tried again and it went 1 -> 3. 🤨

@holgerd77 If 3 was printed then you are using the js version. Can you also print err? This would be very helpful.

I am still a bit unsettled since I was not able yet to find any __napi__ entries in the flamegraph

I may have generated some confusion. The napi frames are tiny and took me a while to find them. But if 3 is printed they won't be there.

What Node version are you actually using? Maybe I'll try run profiling with that version and see if anything changes.

I'm using node v10.22.0 on a fully updated macos installation.

[holgerd77]

I still had a folder renamed which let things break, now I fixed and it is going 1 ->2 again, sorry for all the chaos.

[alcuadrado]

No problem, I'm relieved that it's working as expected :)