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

Issue 2317044: code review 2317044: Optimization of the frequently used strequal. Checks th... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years, 2 months ago by gmiller
Modified:
15 years, 1 month ago
Reviewers:
CC:
rsc, cwvh, golang-dev
Visibility:
Public.

Description

Optimization of the frequently used strequal. Checks the relative length of the two strings, then delegates to memequal. memequal has been changed to use pointer math.

Patch Set 1 #

Total comments: 2

Patch Set 2 : code review 2317044: Optimization of the frequently used strequal. Checks th... #

Patch Set 3 : code review 2317044: Optimization of the frequently used strequal. Checks th... #

Total comments: 1

Patch Set 4 : code review 2317044: Optimization of the frequently used strequal. Checks th... #

Patch Set 5 : code review 2317044: Optimization of the frequently used strequal. Checks th... #

Total comments: 2

Patch Set 6 : code review 2317044: Optimization of the frequently used strequal. Checks th... #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -4 lines) Patch
M src/pkg/runtime/runtime.c View 1 2 3 4 5 2 chunks +13 lines, -4 lines 0 comments Download

Messages

Total messages: 13
bsiegert
http://codereview.appspot.com/2317044/diff/1/src/pkg/runtime/runtime.c File src/pkg/runtime/runtime.c (right): http://codereview.appspot.com/2317044/diff/1/src/pkg/runtime/runtime.c#newcode319 src/pkg/runtime/runtime.c:319: while (ba != aend) { Just a nitpick: no ...
15 years, 2 months ago (2010-10-06 09:27:57 UTC) #1
gmiller
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change.
15 years, 2 months ago (2010-10-06 13:03:28 UTC) #2
gmiller
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), Please take another look.
15 years, 2 months ago (2010-10-06 13:25:24 UTC) #3
rsc1
few more nits http://codereview.appspot.com/2317044/diff/8001/src/pkg/runtime/runtime.c File src/pkg/runtime/runtime.c (right): http://codereview.appspot.com/2317044/diff/8001/src/pkg/runtime/runtime.c#newcode397 src/pkg/runtime/runtime.c:397: int32 aLen = a->len; style is ...
15 years, 2 months ago (2010-10-06 14:05:39 UTC) #4
gmiller
Hello rsc (cc: golang-dev@googlegroups.com), Please take another look.
15 years, 2 months ago (2010-10-06 14:18:16 UTC) #5
rsc
looking better, but you missed two of my comments: blank line before USED(s). s/aLen/alen/ russ
15 years, 2 months ago (2010-10-06 14:36:29 UTC) #6
gmiller
Sorry, not enough coffee this morning. Will resubmit. (fingers crossed) On 2010/10/06 14:36:29, rsc wrote: ...
15 years, 2 months ago (2010-10-06 14:47:00 UTC) #7
gmiller
Hello rsc (cc: golang-dev@googlegroups.com), Please take another look.
15 years, 2 months ago (2010-10-06 14:52:28 UTC) #8
rsc
LGTM Will submit once I've added you to the contributors list. Thanks.
15 years, 2 months ago (2010-10-06 15:13:44 UTC) #9
cwvh
http://codereview.appspot.com/2317044/diff/3/src/pkg/runtime/runtime.c File src/pkg/runtime/runtime.c (right): http://codereview.appspot.com/2317044/diff/3/src/pkg/runtime/runtime.c#newcode402 src/pkg/runtime/runtime.c:402: return memequal(alen, a->str, b->str); Why introduce the redundant variable ...
15 years, 2 months ago (2010-10-06 17:29:39 UTC) #10
gmiller
Hello rsc, cwvh (cc: golang-dev@googlegroups.com), Please take another look.
15 years, 2 months ago (2010-10-06 17:46:34 UTC) #11
gmiller
http://codereview.appspot.com/2317044/diff/3/src/pkg/runtime/runtime.c File src/pkg/runtime/runtime.c (right): http://codereview.appspot.com/2317044/diff/3/src/pkg/runtime/runtime.c#newcode402 src/pkg/runtime/runtime.c:402: return memequal(alen, a->str, b->str); On 2010/10/06 17:29:39, cwvh wrote: ...
15 years, 2 months ago (2010-10-06 17:53:43 UTC) #12
rsc
15 years, 2 months ago (2010-10-07 07:13:27 UTC) #13
*** Submitted as http://code.google.com/p/go/source/detail?r=3be74abce2a4 ***

runtime: faster strequal, memequal

Fixes issue 1161.

R=rsc, cwvh
CC=golang-dev
http://codereview.appspot.com/2317044

Committer: Russ Cox <rsc@golang.org>
Sign in to reply to this message.

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