Resolve #88

[nathanielhourt]

Change Description

Implement the CDT API wrapper for get_code_hash intrinsic and add some testing

API Changes

• API Changes

Added get_code_hash intrinsic to contracts' C and C++ APIs

Documentation Additions

• Documentation Additions

[larryk85]

Appreciate the PR Nathan.

[dimas1185]

not sure which one is more efficient but we have a common pattern to execute intrinsic where first we execute it with zero size first, it returns with return structure size and then we execute it again. example is get_action:

   inline action get_action( uint32_t type, uint32_t index ) {
      constexpr size_t max_stack_buffer_size = 512;
      int s = internal_use_do_not_use::get_action( type, index, nullptr, 0 );
      eosio::check( s > 0, "get_action size failed" );
      size_t size = static_cast<size_t>(s);
      char* buffer = (char*)( max_stack_buffer_size < size ? malloc(size) : alloca(size) );
      auto size2 = internal_use_do_not_use::get_action( type, index, buffer, size );
      eosio::check( size == static_cast<size_t>(size2), "get_action failed" );
      return eosio::unpack<eosio::action>( buffer, size );
   }

[nathanielhourt]

Hmm, well I'm not sure how the overheads stack up in the WASM VM, but calling an intrinsic is roughly equivalent in principle to a syscall to kernel space from userland, and those are notoriously slow. alloca, on the other hand, is merely ticking a register, which is pretty much the fastest instruction imaginable.

There's no substitute for benchmarking, but I would expect making an extra call out of the VM to be at least a couple of orders of magnitude slower than alloca. I think it would only make sense to make the second call if there was a possibility that the struct would be unpredictably large (especially, structs containing string or vector, etc), but even in this case, just allocating a very large heap buffer is probably much faster, just more wasteful of space. That's a hard decision for a library to make, since different clients have different constraints, but in this case the buffer's size has a tight upper bound so we don't need to concern ourselves. =)

[nathanielhourt]

Is there a way to get microseconds or CPU ticks within a contract? I'd like to run some benchmarks, including the cost of calling an intrinsic, as I'm pretty certain it falls in the category of obnoxiously slow, and it might be worth my effort to go through and replace all of these "when you call the intrinsic once, call it twice" instances with something way faster. CPU time is scarce on the blockchain, so the stdlib should make an effort to conserve it for the contract.

[heifner]

Is there a way to get microseconds or CPU ticks within a contract?

Seems like that would be a good addition to: eosnetworkfoundation/product#134

[dimas1185]

@nathanielhourt I agree with your point but the easiest solution looks to me just to make it inline with other stuff. Anyway there are not a lot. If you can spend extra and tell the difference, we can go that way and redo this (probably as part of other PR).
@larryk85 any thought on that?

[spoonincode]

Since the packed size will always be the same unless we somehow manage to make it to 128 versions of the struct, which seems incredibly unlikely since each bump would be a consensus change, it does seem rather wasteful to make multiple calls. It seems unlikely to even need to *2 the size here.

I might suggest checking the return value of internal_use_do_not_use::get_code_hash(), maybe even in lieu of the version check (the possibility that the host function could in the future return a version different than the one asked for would seem to be devastating to existing contracts -- why would we ever do that).

[nathanielhourt]

Hahaha yeah, the version check is abysmally pessimistic, but I like to make such checks where cheap. Bugs love to hide at API boundaries and deucedly so at versioned ones! =)

The *2 size is definitely overkill, and I would be amenable to reducing it. I also agree that checking the return value is prudent. Eventually, a general template could probably be improvised to do an optimistic fast path (alloca and single call) with a slow path fallback (malloc and second call) if the first call didn't allocate adequate space, and even give motivated clients the option to pass a size hint to optimize the fast path for their case, and make that template reusable for all the intrinsics. That would be nice. But for now, I can just go for a retval check and fallback slow path, and maybe dress it up to a template in a future PReq.

[nathanielhourt]

OK, check out the new version. Also, I just noticed that the get_action implementation has a memory leak in the greater than 512 byte case.

[dimas1185]

@nathanielhourt PR has been approved. Thanks. Do you want to add something or just letting enf dev merge by themselves? normally developer pushes their changes so you are good to go if you are done

[nathanielhourt]

Oh, I don't have privs to merge it even after approval. I figured someone else would do it. =)

[nathanielhourt]

Appreciate the PR Nathan

Call me Nate =)