Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump proj4 version to fix multiple performance issues #3039

Merged
merged 6 commits into from
Aug 24, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions bench/src/main/scala/geotrellis/bench/package.scala
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
/*
* Copyright 2019 Azavea
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package geotrellis

import geotrellis.raster.io.geotiff.SinglebandGeoTiff
Expand Down
116 changes: 116 additions & 0 deletions bench/src/main/scala/geotrellis/proj4/CRSBench.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/*
* Copyright 2019 Azavea & Astraea, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*
* This file is a bit modified version of https://github.com/locationtech/rasterframes/blob/dc561dc9ad49eca043a6a0465f11d18d09566db1/bench/src/main/scala/org/locationtech/rasterframes/bench/CRSBench.scala
*/

package geotrellis.proj4

import java.util.concurrent.TimeUnit

import org.locationtech.proj4j.CoordinateReferenceSystem
import org.openjdk.jmh.annotations._

@BenchmarkMode(Array(Mode.AverageTime))
@State(Scope.Benchmark)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
class CRSBench {

var crs1: CRS = _
var crs2: CRS = _

/**
* jmh:run -i 3 -wi 3 -f1 -t1 .*CRSBench.*
*
* LazyCRS (rasterframes fix):
* [info] Benchmark Mode Cnt Score Error Units
* [info] CRSBench.logicalEqualsFalse avgt 3 13.534 ± 6.396 us/op
* [info] CRSBench.logicalEqualsTrue avgt 3 0.254 ± 0.116 us/op
* [info] CRSBench.logicalLazyEqualsFalse avgt 3 14.505 ± 10.106 us/op
* [info] CRSBench.logicalLazyEqualsTrue avgt 3 0.035 ± 0.007 us/op
* [info] CRSBench.resolveCRS avgt 3 0.069 ± 0.082 us/op
*
* Proj4 1.0.0:
* [info] Benchmark Mode Cnt Score Error Units
* [info] CRSBench.logicalEqualsFalse avgt 3 23.397 ± 37.290 us/op
* [info] CRSBench.logicalEqualsTrue avgt 3 11.639 ± 49.851 us/op
* [info] CRSBench.logicalLazyEqualsFalse avgt 3 27.671 ± 106.851 us/op
* [info] CRSBench.logicalLazyEqualsTrue avgt 3 12.928 ± 28.639 us/op
* [info] CRSBench.resolveCRS avgt 3 0.233 ± 0.844 us/op
*
* GeoTools:
* [info] Benchmark Mode Cnt Score Error Units
* [info] CRSBench.logicalEqualsFalse avgt 3 0.046 ± 0.068 us/op
* [info] CRSBench.logicalEqualsTrue avgt 3 0.042 ± 0.043 us/op
* [info] CRSBench.logicalLazyEqualsFalse avgt 3 0.047 ± 0.052 us/op
* [info] CRSBench.logicalLazyEqualsTrue avgt 3 0.042 ± 0.028 us/op
* [info] CRSBench.resolveCRS avgt 3 0.041 ± 0.127 us/op
*
* Proj4 1.0.1 (backed by Caffeine, unbounded):
* [info] Benchmark Mode Cnt Score Error Units
* [info] CRSBench.epsgCodeReadTest avgt 3 0.062 ± 0.032 us/op
* [info] CRSBench.logicalEqualsFalse avgt 3 0.037 ± 0.009 us/op
* [info] CRSBench.logicalEqualsTrue avgt 3 0.054 ± 0.013 us/op
* [info] CRSBench.logicalVarEqualsFalse avgt 3 0.038 ± 0.015 us/op
* [info] CRSBench.logicalVarEqualsTrue avgt 3 0.053 ± 0.022 us/op
* [info] CRSBench.resolveCRS avgt 3 0.035 ± 0.025 us/op
*
* Proj4 1.0.1 (backed by ConcurrentHashMap):
* [info] Benchmark Mode Cnt Score Error Units
* [info] CRSBench.getEpsgCodeTest avgt 3 0.064 ± 0.161 us/op
* [info] CRSBench.logicalEqualsFalse avgt 3 0.039 ± 0.009 us/op
* [info] CRSBench.logicalEqualsTrue avgt 3 0.035 ± 0.020 us/op
* [info] CRSBench.logicalVarEqualsFalse avgt 3 0.040 ± 0.068 us/op
* [info] CRSBench.logicalVarEqualsTrue avgt 3 0.037 ± 0.034 us/op
* [info] CRSBench.resolveCRS avgt 3 0.034 ± 0.008 us/op
*/

@Setup(Level.Invocation)
def setupData(): Unit = {
crs1 = CRS.fromEpsgCode(4326)
crs2 = WebMercator
}

@Benchmark
def resolveCRS(): CoordinateReferenceSystem = {
crs1.proj4jCrs
}

@Benchmark
def logicalEqualsTrue(): Boolean = {
crs1 == LatLng
}

@Benchmark
def logicalEqualsFalse(): Boolean = {
crs1 == WebMercator
}

@Benchmark
def logicalVarEqualsTrue(): Boolean = {
crs1 == crs1
}

@Benchmark
def logicalVarEqualsFalse(): Boolean = {
crs1 == crs2
}

@Benchmark
def getEpsgCodeTest(): Boolean = {
CRS.getEpsgCode("+proj=longlat +ellps=intl +towgs84=84,274,65,0,0,0,0 +no_defs").get == 4630
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package geotrellis.raster

import geotrellis.bench.init
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package geotrellis.raster.render

import org.openjdk.jmh.annotations._
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
/*
* Copyright 2019 Azavea
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package geotrellis.raster.reproject

import org.openjdk.jmh.annotations._
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

package geotrellis.spark.store.index
import geotrellis.store.index.MergeQueue

import org.openjdk.jmh.annotations._

Expand Down
1 change: 1 addition & 0 deletions docs/CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ Fixes & Updates
- Refactor IO thread pool usage (`#3007 <https://github.com/locationtech/geotrellis/pull/3007>`_).
- ``S3RangeReader`` will now optionally read the HEADER of an object (`#3025 <https://github.com/locationtech/geotrellis/pull/3025>`_).
- ``FileRangeReaderProvider`` can now handle more types of ``URI``\s (`#3034 <https://github.com/locationtech/geotrellis/pull/3034>`_).
- Bump proj4 version to fix multiple performance issues (`#3039 <https://github.com/locationtech/geotrellis/pull/3039>`_).

2.3.0
-----
Expand Down
108 changes: 15 additions & 93 deletions proj4/src/main/scala/geotrellis/proj4/CRS.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,12 @@ package geotrellis.proj4
import geotrellis.proj4.io.wkt.WKT

import org.locationtech.proj4j._
import com.github.blemale.scaffeine.Scaffeine
import org.locationtech.proj4j.util.CRSCache

import scala.io.Source
import scala.util.Try


object CRS {
private lazy val proj4ToEpsgMap =
Scaffeine()
.recordStats()
.build[String, Option[String]]()

// new Memoize[String, Option[String]](readEpsgCodeFromFile)
private val crsFactory = new CRSFactory
private val filePrefix = "/proj4/nad/"
private val crsFactory = new CRSCache()

/**
* Creates a CoordinateReferenceSystem
Expand All @@ -48,15 +39,12 @@ object CRS {
* @return The specified CoordinateReferenceSystem
*/
def fromString(proj4Params: String): CRS =
new CRS {
val proj4jCrs: CoordinateReferenceSystem = crsFactory.createFromParameters(null, proj4Params)
}
new CRS { val proj4jCrs: CoordinateReferenceSystem = crsFactory.createFromParameters(null, proj4Params) }

/**
* Returns the numeric EPSG code of a proj4string.
*/
def getEpsgCode(proj4String: String): Option[Int] =
proj4ToEpsgMap.get(proj4String, { key => readEpsgCodeFromFile(key) }).map(_.toInt)
def getEpsgCode(proj4String: String): Option[Int] = readEpsgFromParameters(proj4String).map(_.toInt)

/**
* Creates a CoordinateReferenceSystem
Expand All @@ -72,17 +60,14 @@ object CRS {
* @return The specified CoordinateReferenceSystem
*/
def fromString(name: String, proj4Params: String): CRS =
new CRS {
val proj4jCrs: CoordinateReferenceSystem = crsFactory.createFromParameters(name, proj4Params)
}
new CRS { val proj4jCrs: CoordinateReferenceSystem = crsFactory.createFromParameters(name, proj4Params) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In proj4 1.1 or 2.0 (whatever), can we consider making these case classes instead of anonymous ones? Make for a bit more natural serialization, easier debugging, etc.

Copy link
Member Author

@pomadchin pomadchin Jul 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@metasim it would be a geotrellis change, sure I'll do it 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@metasim do you have any preferred idea / class hierarchy for the entire CRS story?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pomadchin I hadn't thought about it until you asked... how's this for a strawdog:

image

When fromWKT or fromProj4String or getEpsgCode you'd get an instance of one of the intermediate classes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@metasim can you bring this conversation into a separate issue?


/**
* Creates a CoordinateReferenceSystem (CRS) from a
* well-known-text String.
*/
def fromWKT(wktString: String): CRS = {
val epsgCode: String = WKT.getEpsgCode(wktString)

fromName(epsgCode)
}

Expand Down Expand Up @@ -112,39 +97,15 @@ object CRS {
* @return The CoordinateReferenceSystem corresponding to the given name
*/
def fromName(name: String): CRS =
new CRS {
val proj4jCrs: CoordinateReferenceSystem = crsFactory.createFromName(name)
}
new CRS { val proj4jCrs: CoordinateReferenceSystem = crsFactory.createFromName(name) }

/**
* Creates a CoordinateReferenceSystem (CRS) from an EPSG code.
*/
def fromEpsgCode(epsgCode: Int) =
fromName(s"EPSG:$epsgCode")

private def readEpsgCodeFromFile(proj4String: String): Option[String] = {
// TODO: move this functinality to org.locationtech.proj4j.Proj4FileReaeder
val stream = crsFactory.getClass.getResourceAsStream(s"${filePrefix}epsg")

try {
Source.fromInputStream(stream)
.getLines
.find { line =>
!line.startsWith("#") && {
// FIX this match is sensative to white spaces and ordering
val proj4Body = line.split("proj")(1)
s"+proj$proj4Body" == proj4String
}
}.flatMap { l =>
val array = l.split(" ")
val length = array(0).length
// read the int ...
Some(array(0).substring(1, length - 1))
}
} finally {
stream.close()
}
}
def fromEpsgCode(epsgCode: Int) = fromName(s"EPSG:$epsgCode")

def readEpsgFromParameters(proj4String: String): Option[String] =
Option(crsFactory.readEpsgFromParameters(proj4String))

/** Mix-in for singleton CRS implementations where distinguished string should be the name of the object. */
private[proj4] trait ObjectNameToString { self: CRS ⇒
Expand All @@ -159,10 +120,8 @@ trait CRS extends Serializable {

def epsgCode: Option[Int] = {
proj4jCrs.getName.split(":") match {
case Array(name, code) if name.toUpperCase == "EPSG" =>
Try(code.toInt).toOption
case _ =>
CRS.getEpsgCode(toProj4String + " <>")
case Array(name, code) if name.toUpperCase == "EPSG" => Try(code.toInt).toOption
case _ => CRS.getEpsgCode(toProj4String + " <>")
}
}

Expand All @@ -184,55 +143,18 @@ trait CRS extends Serializable {


// TODO: Do these better once more things are ported
override
def hashCode = toProj4String.hashCode
override def hashCode = toProj4String.hashCode

def toProj4String: String = proj4jCrs.getParameterString

def isGeographic: Boolean = proj4jCrs.isGeographic

override
def equals(o: Any): Boolean =
override def equals(o: Any): Boolean =
o match {
case other: CRS => compareProj4Strings(other.toProj4String, toProj4String)
case other: CRS => proj4jCrs == other.proj4jCrs
case _ => false
}

private def compareProj4Strings(p1: String, p2: String) = {
def toProj4Map(s: String): Map[String, String] =
s.trim.split(" ").map(x =>
if (x.startsWith("+")) x.substring(1) else x).map(x => {
val index = x.indexOf('=')
if (index != -1) (x.substring(0, index) -> Some(x.substring(index + 1)))
else (x -> None)
}).groupBy(_._1).map { case (a, b) => (a, b.head._2) }
.filter { case (a, b) => !b.isEmpty }.map { case (a, b) => (a -> b.get) }
.map { case (a, b) => if (b == "latlong") (a -> "longlat") else (a, b) }
.filter { case (a, b) => (a != "to_meter" || b != "1.0") }

val m1 = toProj4Map(p1)
val m2 = toProj4Map(p2)

m1.map {
case (key, v1) => m2.get(key) match {
case Some(v2) => compareValues(v1, v2)
case None => false
}
}.forall(_ != false)
}

private def compareValues(s1: String, s2: String) = {
def isNumber(s: String) = s.filter(c => !List('.', '-').contains(c)) forall Character.isDigit

val s2IsNumber = isNumber(s1)
val s1IsNumber = isNumber(s2)

if (s1IsNumber == s2IsNumber) {
if (s1IsNumber) math.abs(s1.toDouble - s2.toDouble) < Epsilon
else s1 == s2
} else false
}

protected def factory = CRS.crsFactory

/** Default implementation returns the proj4 name. */
Expand Down
2 changes: 1 addition & 1 deletion proj4/src/main/scala/geotrellis/proj4/ConusAlbers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ import geotrellis.proj4.CRS.ObjectNameToString
object ConusAlbers extends CRS with ObjectNameToString {
lazy val proj4jCrs = factory.createFromName("EPSG:5070")

override def epsgCode: Option[Int] = Some(5070)
override val epsgCode: Option[Int] = Some(5070)
}
2 changes: 1 addition & 1 deletion proj4/src/main/scala/geotrellis/proj4/LatLng.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ import geotrellis.proj4.CRS.ObjectNameToString
object LatLng extends CRS with ObjectNameToString {
lazy val proj4jCrs = factory.createFromName("EPSG:4326")

override def epsgCode: Option[Int] = Some(4326)
override val epsgCode: Option[Int] = Some(4326)
}
2 changes: 1 addition & 1 deletion proj4/src/main/scala/geotrellis/proj4/WebMercator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ import geotrellis.proj4.CRS.ObjectNameToString
object WebMercator extends CRS with ObjectNameToString {
lazy val proj4jCrs = factory.createFromName("EPSG:3857")

override def epsgCode: Option[Int] = Some(3857)
override val epsgCode: Option[Int] = Some(3857)
}
2 changes: 1 addition & 1 deletion project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ object Dependencies {
val scalacheck = "org.scalacheck" %% "scalacheck" % "1.14.0"
val scalaXml = "org.scala-lang.modules" %% "scala-xml" % "1.2.0"
val jts = "org.locationtech.jts" % "jts-core" % "1.16.1"
val proj4j = "org.locationtech.proj4j" % "proj4j" % "1.0.0"
val proj4j = "org.locationtech.proj4j" % "proj4j" % "1.0.1-SNAPSHOT"

val monocleCore = "com.github.julien-truffaut" %% "monocle-core" % Version.monocle
val monocleMacro = "com.github.julien-truffaut" %% "monocle-macro" % Version.monocle
Expand Down
Loading