Skip to content

Commit

Permalink
Optimizations: Only link used datasets, avoid copies, avoid pkg/error…
Browse files Browse the repository at this point in the history
…s dependency (#26)

* Run go fmt

* Ignore GoLand IDE directory

* Let linker strip unused datasets

This is achieved by making each dataset a separatge variable, which
can be stripped out by the linker when it is no referenced anywhere.
Also avoids unnecessary copies of gzipped dataset.

* Remove github.com/pkg/errors dependency

* Update dependencies

* Use Go 1.21 in CI

Go version in go.mod can remain at 1.19 for compatibility.

* Preallocate slices
  • Loading branch information
mologie authored Nov 21, 2023
1 parent e3d9883 commit f725e83
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 69 deletions.
12 changes: 6 additions & 6 deletions .github/workflows/continuous-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ jobs:
ubuntu:
runs-on: ubuntu-latest
steps:
- name: Set up Go 1.19
- name: Set up Go 1.21
uses: actions/setup-go@v1
with:
go-version: 1.19.x
go-version: 1.21.x
id: go

- name: Check out code into the Go module directory
Expand All @@ -38,10 +38,10 @@ jobs:
macOS:
runs-on: macos-latest
steps:
- name: Set up Go 1.19
- name: Set up Go 1.21
uses: actions/setup-go@v1
with:
go-version: 1.19.x
go-version: 1.21.x
id: go

- name: Check out code into the Go module directory
Expand All @@ -59,10 +59,10 @@ jobs:
windows:
runs-on: windows-latest
steps:
- name: Set up Go 1.19
- name: Set up Go 1.21
uses: actions/setup-go@v1
with:
go-version: 1.19.x
go-version: 1.21.x
id: go

- name: Check out code into the Go module directory
Expand Down
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@
# Output of the go coverage tool, specifically when used with LiteIDE
*.out

# IDEs
.idea/

# Dependency directories (remove the comment below to include it)
# vendor/

*.geojson
datagen/datagen
datagen/datagen
38 changes: 17 additions & 21 deletions embed.go
Original file line number Diff line number Diff line change
@@ -1,36 +1,32 @@
package rgeo

import (
"embed"
"fmt"
"io"
)
// embedding files individually here to allow the linker to strip out unused ones
import _ "embed"

//go:embed data/*
var dataDir embed.FS
//go:embed data/Cities10.gz
var cities10 []byte

func Cities10() []byte {
return readEmbedFile("Cities10.gz")
return cities10
}

//go:embed data/Countries10.gz
var countries10 []byte

func Countries10() []byte {
return readEmbedFile("Countries10.gz")
return countries10
}

func Countries110() []byte {
return readEmbedFile("Countries110.gz")
}
//go:embed data/Countries110.gz
var countries110 []byte

func Provinces10() []byte {
return readEmbedFile("Provinces10.gz")
func Countries110() []byte {
return countries110
}

func readEmbedFile(name string) []byte {
f, err := dataDir.Open(fmt.Sprintf("data/%s", name))
if err != nil {
panic(fmt.Sprintf("internal embedded geojson open error %v", err))
}
//go:embed data/Provinces10.gz
var provinces10 []byte

b, _ := io.ReadAll(f)
return b
func Provinces10() []byte {
return provinces10
}
5 changes: 2 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ go 1.19

require (
github.com/go-test/deep v1.1.0
github.com/golang/geo v0.0.0-20200730024412-e86565bf3f35
github.com/pkg/errors v0.9.1
github.com/twpayne/go-geom v1.4.4
github.com/golang/geo v0.0.0-20230421003525-6adc56603217
github.com/twpayne/go-geom v1.5.2
)
10 changes: 4 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/go-test/deep v1.1.0 h1:WOcxcdHcvdgThNXjw0t76K42FXTU7HpNQWHpA2HHNlg=
github.com/go-test/deep v1.1.0/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE=
github.com/golang/geo v0.0.0-20200730024412-e86565bf3f35 h1:enTowfyfjtomBQhxX9mhUD+0tZhpe4rIzStO4aNlou8=
github.com/golang/geo v0.0.0-20200730024412-e86565bf3f35/go.mod h1:QZ0nwyI2jOfgRAoBvP+ab5aRr7c9x7lhGEJrKvBwjWI=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/golang/geo v0.0.0-20230421003525-6adc56603217 h1:HKlyj6in2JV6wVkmQ4XmG/EIm+SCYlPZ+V4GWit7Z+I=
github.com/golang/geo v0.0.0-20230421003525-6adc56603217/go.mod h1:8wI0hitZ3a1IxZfeH3/5I97CI8i5cLGsYe7xNhQGs9U=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
github.com/twpayne/go-geom v1.4.4 h1:bcCPAvvNSzjmpUqR0Uqh39ClCKtPx6kZVR7EakQaVJI=
github.com/twpayne/go-geom v1.4.4/go.mod h1:Kz4sX4LtdesDQgkhsMERazLlH/NiCg90s6FPaNr0KNI=
github.com/twpayne/go-geom v1.5.2 h1:LyRfBX2W0LM7XN/bGqX0XxrJ7SZc3XwmxU4aj4kSoxw=
github.com/twpayne/go-geom v1.5.2/go.mod h1:3z6O2sAnGtGCXx4Q+5nPOLCA5e8WI2t3cthdb1P2HH8=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
43 changes: 20 additions & 23 deletions rgeo.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ to be near specific borders I would advise checking the data beforehand (links
to which are in the files). If you want to use your own dataset, check out the
datagen folder.
Installation
# Installation
go get github.com/sams96/rgeo
Contributing
# Contributing
Contributions are welcome, I haven't got any guidelines or anything so maybe
just make an issue first.
Expand All @@ -40,17 +40,18 @@ import (
"bytes"
"compress/gzip"
"encoding/json"
"errors"
"fmt"
"strings"

"github.com/golang/geo/s2"
"github.com/pkg/errors"
geom "github.com/twpayne/go-geom"
"github.com/twpayne/go-geom"
"github.com/twpayne/go-geom/encoding/geojson"
)

// ErrLocationNotFound is returned when no country is found for given
// coordinates.
var ErrLocationNotFound = errors.Errorf("country not found")
var ErrLocationNotFound = errors.New("country not found")

// Location is the return type for ReverseGeocode.
type Location struct {
Expand Down Expand Up @@ -101,29 +102,25 @@ func New(datasets ...func() []byte) (*Rgeo, error) {
// Parse GeoJSON
var fc geojson.FeatureCollection

for _, dataset := range datasets {
dec := dataset()

if len(dec) <= 0 {
return nil, errors.New("invalid data: no data found")
for i, dataset := range datasets {
br := bytes.NewReader(dataset())
if br.Len() == 0 {
return nil, fmt.Errorf("no data in dataset %d", i)
}

var b bytes.Buffer
b.Write(dec)

zr, err := gzip.NewReader(&b)
zr, err := gzip.NewReader(br)
if err != nil {
return nil, errors.Wrap(err, "invalid dataset")
return nil, fmt.Errorf("decompression failed for dataset %d: %w", i, err)
}

// Parse GeoJSON
var tfc geojson.FeatureCollection
if err := json.NewDecoder(zr).Decode(&tfc); err != nil {
return nil, errors.Wrap(err, "invalid dataset: JSON")
return nil, fmt.Errorf("invalid JSON in dataset %d: %w", i, err)
}

if err := zr.Close(); err != nil {
return nil, errors.Wrap(err, "failed to close gzip reader")
return nil, fmt.Errorf("failed to close gzip reader for dataset %d: %w", i, err)
}

fc.Features = append(fc.Features, tfc.Features...)
Expand All @@ -138,7 +135,7 @@ func New(datasets ...func() []byte) (*Rgeo, error) {
for _, c := range fc.Features {
p, err := polygonFromGeometry(c.Geometry)
if err != nil {
return nil, errors.Wrap(err, "invalid dataset")
return nil, fmt.Errorf("bad polygon in geometry: %w", err)
}

ret.index.Add(p)
Expand Down Expand Up @@ -243,7 +240,7 @@ func polygonFromGeometry(g geom.T) (*s2.Polygon, error) {
case *geom.MultiPolygon:
polygon, err = polygonFromMultiPolygon(t)
default:
return nil, errors.Errorf("needs Polygon or MultiPolygon")
return nil, errors.New("needs Polygon or MultiPolygon")
}

if err != nil {
Expand All @@ -255,7 +252,7 @@ func polygonFromGeometry(g geom.T) (*s2.Polygon, error) {

// Converts a geom MultiPolygon to an s2 Polygon.
func polygonFromMultiPolygon(p *geom.MultiPolygon) (*s2.Polygon, error) {
var loops []*s2.Loop
loops := make([]*s2.Loop, 0, p.NumPolygons())

for i := 0; i < p.NumPolygons(); i++ {
this, err := loopSliceFromPolygon(p.Polygon(i))
Expand All @@ -279,18 +276,18 @@ func polygonFromPolygon(p *geom.Polygon) (*s2.Polygon, error) {
//
// Modified from types.loopFromPolygon from github.com/dgraph-io/dgraph.
func loopSliceFromPolygon(p *geom.Polygon) ([]*s2.Loop, error) {
var loops []*s2.Loop
loops := make([]*s2.Loop, 0, p.NumLinearRings())

for i := 0; i < p.NumLinearRings(); i++ {
r := p.LinearRing(i)
n := r.NumCoords()

if n < 4 {
return nil, errors.Errorf("can't convert ring with less than 4 points")
return nil, errors.New("can't convert ring with less than 4 points")
}

if !r.Coord(0).Equal(geom.XY, r.Coord(n-1)) {
return nil, errors.Errorf(
return nil, fmt.Errorf(
"last coordinate not same as first for polygon: %+v", p.FlatCoords())
}

Expand Down
18 changes: 9 additions & 9 deletions rgeo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ func TestNew_BadData(t *testing.T) {
{
name: "Empty data",
in: func() []byte { return []byte(``) },
err: "invalid data: no data found",
err: "no data in dataset 0",
},
{
name: "Wrong type",
Expand All @@ -361,7 +361,7 @@ func TestNew_BadData(t *testing.T) {
{"type":"Point","coordinates":[0,0]}}]}`,
)
},
err: "invalid dataset: needs Polygon or MultiPolygon",
err: "bad polygon in geometry: needs Polygon or MultiPolygon",
},
{
name: "Small polygon",
Expand All @@ -373,7 +373,7 @@ func TestNew_BadData(t *testing.T) {
"coordinates":[[[1,2],[3,4],[1,2]]]}}]}`,
)
},
err: "invalid dataset: can't convert ring with less than 4 points",
err: "bad polygon in geometry: can't convert ring with less than 4 points",
},
{
name: "No repeated end",
Expand All @@ -385,8 +385,8 @@ func TestNew_BadData(t *testing.T) {
"coordinates":[[[1,2],[3,4],[5,6],[7,8]]]}}]}`,
)
},
err: "invalid dataset: last coordinate not same as first for " +
"polygon: [1 2 3 4 5 6 7 8]",
err: "bad polygon in geometry: " +
"last coordinate not same as first for polygon: [1 2 3 4 5 6 7 8]",
},
{
name: "Bad Multipolygon",
Expand All @@ -398,18 +398,18 @@ func TestNew_BadData(t *testing.T) {
"coordinates":[[[[1,2],[3,4],[5,6],[7,8]]]]}}]}`,
)
},
err: "invalid dataset: last coordinate not same as first for " +
"polygon: [1 2 3 4 5 6 7 8]",
err: "bad polygon in geometry: " +
"last coordinate not same as first for polygon: [1 2 3 4 5 6 7 8]",
},
{
name: "Bad compression",
in: func() []byte { return []byte(`dGhpcyBpcyBub3QgU29tcHJIc3NIZA==`) },
err: "invalid dataset: gzip: invalid header",
err: "decompression failed for dataset 0: gzip: invalid header",
},
{
name: "Bad JSON",
in: func() []byte { return []byte(compressData(t, `this is not JSON`)) },
err: "invalid dataset: JSON: invalid character 'h' in literal true (expecting 'r')",
err: "invalid JSON in dataset 0: invalid character 'h' in literal true (expecting 'r')",
},
}
for _, test := range testdata {
Expand Down

0 comments on commit f725e83

Please sign in to comment.