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

Issue 78870047: code review 78870047: unicode: rearrange static data to decrease binary size (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 2 months ago by josharian
Modified:
5 years, 2 months ago
Visibility:
Public.

Description

unicode: rearrange static data to decrease binary size Using a single, shared backing array across RangeTables reduces binary size by ~33k. No API, godoc, or functional changes. While we're in there, combine repeated var declarations into a var block for prettier gofmt output, and tweak the generated code to be closer to its final, gofmt'd form. I tested that there are no functional changes by confirming that the output of `go run dumptables.go | sort` is unchanged. dumptables.go: http://play.golang.org/p/wqHVlEJYli Fixes issue 7600. Update issue 6853 Rearranging static unicode data cut 33k and made further savings available via issue 7599.

Patch Set 1 #

Patch Set 2 : diff -r e8ca57933a8f https://code.google.com/p/go #

Patch Set 3 : diff -r e8ca57933a8f https://code.google.com/p/go #

Total comments: 12

Patch Set 4 : diff -r e8ca57933a8f https://code.google.com/p/go #

Patch Set 5 : diff -r e8ca57933a8f https://code.google.com/p/go #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+4628 lines, -5442 lines) Patch
M src/pkg/unicode/maketables.go View 1 2 3 4 10 chunks +113 lines, -74 lines 0 comments Download
M src/pkg/unicode/tables.go View 1 5 chunks +4515 lines, -5368 lines 1 comment Download

Messages

Total messages: 13
josharian
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
5 years, 2 months ago (2014-03-21 23:53:22 UTC) #1
dfc
point of order, place Update issue 6853 at the top of the description so the ...
5 years, 2 months ago (2014-03-22 00:30:40 UTC) #2
josharian
I intentionally put that shorter summary at the bottom for issue 6853, figuring that the ...
5 years, 2 months ago (2014-03-22 00:34:46 UTC) #3
dfc
I'm a big fan of including as many details as possible. Let's see what others ...
5 years, 2 months ago (2014-03-22 08:27:15 UTC) #4
josharian
> I'm a big fan of including as many details as possible. Let's see what ...
5 years, 2 months ago (2014-03-24 17:50:45 UTC) #5
bradfitz
R=r Rob likes Unicode and large binaries. On Mar 24, 2014 10:50 AM, <josharian@gmail.com> wrote: ...
5 years, 2 months ago (2014-03-24 17:53:35 UTC) #6
r
The change looks pretty good. I need to think about whether this should wait for ...
5 years, 2 months ago (2014-03-25 02:14:20 UTC) #7
josharian
As always, thanks for the thoughtful review. PTAL. I don't feel strongly about 1.3 vs ...
5 years, 2 months ago (2014-03-25 16:00:15 UTC) #8
bradfitz
https://codereview.appspot.com/78870047/diff/80001/src/pkg/unicode/tables.go File src/pkg/unicode/tables.go (right): https://codereview.appspot.com/78870047/diff/80001/src/pkg/unicode/tables.go#newcode1209 src/pkg/unicode/tables.go:1209: var () could be removed.
5 years, 2 months ago (2014-03-25 18:02:19 UTC) #9
josharian
On 2014/03/25 18:02:19, bradfitz wrote: > https://codereview.appspot.com/78870047/diff/80001/src/pkg/unicode/tables.go > File src/pkg/unicode/tables.go (right): > > https://codereview.appspot.com/78870047/diff/80001/src/pkg/unicode/tables.go#newcode1209 > ...
5 years, 2 months ago (2014-03-25 18:13:38 UTC) #10
r
NOT LGTM After thinking this over and talking to rsc and iant, this is probably ...
5 years, 2 months ago (2014-03-25 23:44:25 UTC) #11
josharian
> NOT LGTM > > After thinking this over and talking to rsc and iant, ...
5 years, 2 months ago (2014-03-26 00:16:57 UTC) #12
josharian
5 years, 2 months ago (2014-03-26 00:19:30 UTC) #13
*** Abandoned ***
Sign in to reply to this message.

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