Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(134)

Issue 8160043: runtime: cache most immediately-computed hash value (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 4 months ago by bradfitz
Modified:
9 years, 2 months ago
Reviewers:
dvyukov
Visibility:
Public.

Description

runtime: cache most immediately-computed hash value Proof of concept. NOT FOR SUBMISSION. Not enough benchmarks, but to show that it's working on string keys at least, and slowing things down when it's not triggering. benchmark old ns/op new ns/op delta BenchmarkNewEmptyMap 144 142 -1.39% BenchmarkHashStringSpeed 42 46 +10.21% BenchmarkHashInt32Speed 34 31 -8.45% BenchmarkHashInt64Speed 31 32 +2.52% BenchmarkHashStringArraySpeed 112 126 +12.50% BenchmarkMegMap 298981 28 -99.99% BenchmarkMegOneMap 24 24 +0.40% BenchmarkMegEmptyMap 4 4 +0.44% BenchmarkSmallStrMap 23 23 +0.00% BenchmarkIntMap 17 17 +0.56% BenchmarkRepeatedLookupStrMapKey32 45 35 -20.22% BenchmarkRepeatedLookupStrMapKey1M 306886 154843 -49.54% For Issue 5147

Patch Set 1 #

Patch Set 2 : diff -r 8f36f2ef721b https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r 8f36f2ef721b https://go.googlecode.com/hg/ #

Total comments: 4

Patch Set 4 : diff -r a3717460e37c https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+109 lines, -10 lines) Patch
M src/pkg/runtime/hashmap.c View 1 2 8 chunks +72 lines, -6 lines 0 comments Download
M src/pkg/runtime/hashmap_fast.c View 1 2 chunks +26 lines, -4 lines 0 comments Download
M src/pkg/runtime/proc.c View 1 2 3 6 chunks +6 lines, -0 lines 0 comments Download
M src/pkg/runtime/runtime.h View 1 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 2
dvyukov
https://codereview.appspot.com/8160043/diff/3001/src/pkg/runtime/proc.c File src/pkg/runtime/proc.c (right): https://codereview.appspot.com/8160043/diff/3001/src/pkg/runtime/proc.c#newcode259 src/pkg/runtime/proc.c:259: mp->lasthash = 0; no need to initialize to zeroes, ...
9 years, 4 months ago (2013-03-29 17:21:00 UTC) #1
bradfitz
9 years, 4 months ago (2013-03-29 17:36:49 UTC) #2
Thanks for the review! (I'm new here.) PTAL

https://codereview.appspot.com/8160043/diff/3001/src/pkg/runtime/proc.c
File src/pkg/runtime/proc.c (right):

https://codereview.appspot.com/8160043/diff/3001/src/pkg/runtime/proc.c#newco...
src/pkg/runtime/proc.c:259: mp->lasthash = 0;
On 2013/03/29 17:21:00, dvyukov wrote:
> no need to initialize to zeroes, mp is initially all zeroes

Done. Thanks! I wasn't sure.

https://codereview.appspot.com/8160043/diff/3001/src/pkg/runtime/proc.c#newco...
src/pkg/runtime/proc.c:976: m->lastlen = 0; // clear the hash cache, in case a
GC occurred earlier
On 2013/03/29 17:21:00, dvyukov wrote:
> it needs to be in runtime.park/goexit/gosched/entersyscall/entersyscallblock
> otherwise (1) GC will see it and (2) hashmap/key can be garbage collected
while
> g is in syscall

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b