Optimizations #430

These are the same changes as my previous pull request. They have been broken down into individual commits by function, with a useful commit message. They optimize profile outliers under a specific test load that represents the load the software sees when it's used for pooled mining, that is, when it gets hundreds of RPC requests (mostly 'getwork') per second.

ACK, looks pretty good to me

ACK, with one remark: if the output buffer of the base64 decoder is 1024 bytes, it supports inputs up to 1365 character (not 512). Maybe a smaller buffer suffices?

Yeah, a smaller buffer or a better test. I was originally going to make the caller pass it a buffer to avoid having to allocate a std:string to return and to permit it to support any length, but it didn't seem worth it since it's only called once per RPC call.

I tried to amend that change without messing up the tree, but my git-fu was insufficient. If I try to revert the original patch, it conflicts in util.h, and I can't find the right way to merge the fix with the revert. And I don't want to clutter the tree with two commits, no with a commit/revert/commit.

Make a new commit in which you fix whatever you want to fix. Then, run git "rebase -i upstream/master", it will show you a list of commits from current master to your HEAD. Modify the line with the fix in it to be "fixup", and possibly move it up if it should not be squashed together with the latest commit.

Thanks. I shrunk the buffer and cleaned up some spacing/indentation issues to better match bitcoin style. I couldn't quite figure out how to get it to squash into the commit that added that code in the first place, but I think I did manage to merge it as a fixup.

Looks good, though why is strRPCUser and strRPCPass defined in util and initialized in init? Could it not be defined and initialized in rpc.cpp so that it doesnt have to be yet another global?

I agree with Matt, strRPCUser/Pass don't belong in util.h.

How much of a speed-up do each of these changes get? (first rule of optimization: measure speed before and after every single change, I can't count the number of times I made a change that "must" make code faster that didn't).

strRPCUser/strRPCPass makes a 'getwork' RPC request about 2% faster alone. However, the improvement is only so small because there are so many other performance disasters in that code path (once you knock those down, it's about 9%). It's true that this specific optimization doesn't help the mainline code very much only because the mainline's RPC and JSON code is so poor.

But those changes are coming. My version is already in my 4diff patches, just not suitable for merge. And there's also a version of the RPC fixes already as a pull request (#214).

Can you avoid using words like "disaster" when you mean "not optimized for what I want to use it for" ?

Sorry for sounding grumpy, but I'm grumpy. Adding 100+ more lines of code for a speedup that nobody but mining pool operators will notice is not a good tradeoff in my mind, because we're having enough trouble making sure the code we already have works properly in all cases.

I think the truth is somewhere between "disaster" and "not optimized for what I want to use it for".

I agree that there aren't that many mining pool operators, but they have a disproprtionate impact on the way the network operates because they are the most likely to choose what transactions get into blocks, which chains to extend, and so on. I think it benefits the safety and stability of the system as a whole if mining pool operators don't have to maintain a large number of patches, each with associated risk, from mainline.

Specific opinions regarding the different commits:

• Fix UNIX-specific thread handle leak: this is a bugfix, should be merged
• Optimize RPC user/pass lookups: if such a simple change can cause a 2% speedup, i have no objections to merging. I agree it doesn't belong in util but in rpc.
• Faster Base64 decoder: using inline code vs. using a library is always a controversial issue, but this is simple enough imho to do internally, after some correctness tests. It doesn't follow the coding standards though, btw.
• Optimize generation of hex output on 'getwork' requests: if you're able to write a faster version for converting to Hex, would it be possible to use it as a base for replacing the current HexStr() entirely? I don't think we need several pieces of code doing the same thing.

Merged the bug fix

RPC user/pass is a 2% speedup-- what's the speedup of the other 2 patches?

I'll have to track down those results, but they're greater than 2% -- the RPC user/pass cache was the smallest of the improvements. (Note that these are percentages in the specific case where you're hammering the code with 'getwork' requests. They shouldn't make any other workloads worse though.)

I've definitely been pushing for patches that serve the mining community. It is a numerically small set of nodes, but very impactful and important for the communal integrity of our network, IMHO. Lacking any better solution, we should look seriously at applying patchsets being universally adopted by mining pool operators.

I was hoping this thread had to do with optimizing the load time of the GUI. On windows 7 x64 it takes a minimum of 30 seconds on my mining rig. It can take longer than a minute on my laptop, and sometimes doesn't load on either.

This is a major inconvenience when you want to use Bitcoins to pay for a pita at Meze grille. Frustrating enough that people testing out the Bitcoin concept might walk away entirely.

Stronger mining is null if we don't have people spending the Bitcoins.

EDIT:

If there is a demand for changes particular to pool operators, perhaps we can offer an alternative repo with optimizations for pooled mining? I think the further we go with the Bitcoin core, the more apparent the different needs between these two distinct groups of client-downloaders.

AFAIK, some pool operators run their own optimized versions of the client. At some point it might be wise to conglomerate their efforts. Keeping a pool client and a standard client will IMHO allow for greater innovation, and less compromise.

Forgive me if this is an optimization that all clients would need in order to be effective, TBH I don't really know what it does...