Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-25 Thread Andres Freund
On 2014-12-24 13:58:41 -0600, Jim Nasby wrote: > This surprises me given that SMHasher shows XXH to be 50% faster than Spooky > for 20 byte keys. Note that there's quite some differences in how smhasher and postgres use the hash functions. The former nearly exclusively exercises the hash function

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-24 Thread Tom Lane
Jim Nasby writes: > On 12/24/14, 10:58 AM, Tom Lane wrote: >> Andres Freund writes: >>> FWIW, I don't believe these results for one second. It's quite plausible >>> that there's a noticeable performance benefit, but a factor of three is >>> completely unrealistic... Could you please recheck? >>

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-24 Thread Jim Nasby
On 12/24/14, 10:58 AM, Tom Lane wrote: Andres Freund writes: On 2014-12-24 00:27:39 -0600, Jim Nasby wrote: pgbench -S -T10 -c 4 -j 4 master: tps = 9556.356145 (excluding connections establishing) tps = 9897.324917 (excluding connections establishing) tps = 9287.286907 (excluding connections e

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-24 Thread Tom Lane
Andres Freund writes: > On 2014-12-24 00:27:39 -0600, Jim Nasby wrote: >> pgbench -S -T10 -c 4 -j 4 >> master: >> tps = 9556.356145 (excluding connections establishing) >> tps = 9897.324917 (excluding connections establishing) >> tps = 9287.286907 (excluding connections establishing) >> tps = 1021

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-24 Thread Andres Freund
On 2014-12-24 00:27:39 -0600, Jim Nasby wrote: > pgbench -S -T10 -c 4 -j 4 > master: > tps = 9556.356145 (excluding connections establishing) > tps = 9897.324917 (excluding connections establishing) > tps = 9287.286907 (excluding connections establishing) > tps = 10210.130833 (excluding connections

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-23 Thread Jim Nasby
On 12/24/14, 12:27 AM, Jim Nasby wrote: There were several select-only runs on both to warm shared_buffers (set to 512MB for this test, and fsync is off). BTW, presumably this ~380M database isn't big enough to show any problems with hash collisions, and I'm guessing you'd need way more memor

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-23 Thread Jim Nasby
On 12/20/14, 2:13 PM, Jim Nasby wrote: On 12/20/14, 11:51 AM, Tom Lane wrote: Andres Freund writes: On 2014-12-19 22:03:55 -0600, Jim Nasby wrote: What I am thinking is not using all of those fields in their raw form to calculate the hash value. IE: something analogous to: hash_any(SharedBuf

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-20 Thread Jim Nasby
On 12/20/14, 11:51 AM, Tom Lane wrote: Andres Freund writes: On 2014-12-19 22:03:55 -0600, Jim Nasby wrote: What I am thinking is not using all of those fields in their raw form to calculate the hash value. IE: something analogous to: hash_any(SharedBufHash, (rot(forkNum, 2) | dbNode) ^ relNo

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-20 Thread Tom Lane
Andres Freund writes: > On 2014-12-19 22:03:55 -0600, Jim Nasby wrote: >> What I am thinking is not using all of those fields in their raw form to >> calculate the hash value. IE: something analogous to: >> hash_any(SharedBufHash, (rot(forkNum, 2) | dbNode) ^ relNode) << 32 | >> blockNum) >> >>

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-19 Thread Andres Freund
On 2014-12-19 22:03:55 -0600, Jim Nasby wrote: > I'm not suggesting we change BufferTag or BufferLookupEnt; clearly we > can't simply throw away any of the fields I was talking about (well, > except possibly tablespace ID. AFAICT that's completely redundant for > searching because relid is UNIQUE).

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-19 Thread Jim Nasby
On 12/19/14, 5:13 PM, Tom Lane wrote: Jim Nasby writes: On 12/18/14, 5:00 PM, Jim Nasby wrote: 2201582 20 -- Mostly LOCALLOCK and Shared Buffer Started looking into this; perhaps https://code.google.com/p/fast-hash/ would be worth looking at, though it requires uint64. It also occurs to

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-19 Thread Tom Lane
"k...@rice.edu" writes: > If we are going to consider changing the hash function, we should > consider something like xxhash which runs at 13.8GB/s on a 2.7GHz > x86_64 for the XXH64 variant and 6.8GB/s for the XXH32 variant which > is double the speed of fast-hash according to the page running on

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-19 Thread k...@rice.edu
On Fri, Dec 19, 2014 at 04:41:51PM -0600, Jim Nasby wrote: > On 12/18/14, 5:00 PM, Jim Nasby wrote: > >2201582 20 -- Mostly LOCALLOCK and Shared Buffer > > Started looking into this; perhaps https://code.google.com/p/fast-hash/ would > be worth looking at, though it requires uint64. > > It also

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-19 Thread Tom Lane
Jim Nasby writes: > On 12/18/14, 5:00 PM, Jim Nasby wrote: >> 2201582 20 -- Mostly LOCALLOCK and Shared Buffer > Started looking into this; perhaps https://code.google.com/p/fast-hash/ would > be worth looking at, though it requires uint64. > It also occurs to me that we're needlessly shoving a

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-19 Thread Jim Nasby
On 12/18/14, 5:00 PM, Jim Nasby wrote: 2201582 20 -- Mostly LOCALLOCK and Shared Buffer Started looking into this; perhaps https://code.google.com/p/fast-hash/ would be worth looking at, though it requires uint64. It also occurs to me that we're needlessly shoving a lot of 0's into the hash

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-18 Thread Jim Nasby
On 12/18/14, 12:48 PM, Tom Lane wrote: I wrote: Here's a proposed patch along this line. I left in oid_hash (in the form of a macro) so that this does not cause any API break for existing third-party modules. However, no callers in our own code directly refer to tag_hash or oid_hash anymore.

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-18 Thread Tom Lane
I wrote: > Here's a proposed patch along this line. I left in oid_hash (in the > form of a macro) so that this does not cause any API break for existing > third-party modules. However, no callers in our own code directly > refer to tag_hash or oid_hash anymore. Committed that version after some

hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)

2014-12-17 Thread Tom Lane
Teodor Sigaev writes: >> -hash_ctl.hash = oid_hash; /* a bit more efficient than tag_hash */ >> +hash_ctl.hash = tag_hash; /* a bit more efficient than tag_hash */ >> >> I think the comment may need removed here. > Thank you, fixed I looked at this patch. It's not right at all here: +