Unverified Commit 376b689b authored by filipe oliveira's avatar filipe oliveira Committed by GitHub
Browse files

Simplified geoAppendIfWithinShape() and removed spurious calls do sdsdup and sdsfree (#11522)



In scenarios in which we have large datasets and the elements are not
contained within the range we do spurious calls do sdsdup and sdsfree.
I.e. instead of pre-creating an sds before we know if we're gonna use it
or not, change the role of geoAppendIfWithinShape to just do geoWithinShape,
and let the caller create the string only when needed.
Co-authored-by: default avatarOran Agra <oran@redislabs.com>
parent cb7447b3
...@@ -60,14 +60,20 @@ geoArray *geoArrayCreate(void) { ...@@ -60,14 +60,20 @@ geoArray *geoArrayCreate(void) {
return ga; return ga;
} }
/* Add a new entry and return its pointer so that the caller can populate /* Add and populate with data a new entry to the geoArray. */
* it with data. */ geoPoint *geoArrayAppend(geoArray *ga, double *xy, double dist,
geoPoint *geoArrayAppend(geoArray *ga) { double score, char *member)
{
if (ga->used == ga->buckets) { if (ga->used == ga->buckets) {
ga->buckets = (ga->buckets == 0) ? 8 : ga->buckets*2; ga->buckets = (ga->buckets == 0) ? 8 : ga->buckets*2;
ga->array = zrealloc(ga->array,sizeof(geoPoint)*ga->buckets); ga->array = zrealloc(ga->array,sizeof(geoPoint)*ga->buckets);
} }
geoPoint *gp = ga->array+ga->used; geoPoint *gp = ga->array+ga->used;
gp->longitude = xy[0];
gp->latitude = xy[1];
gp->dist = dist;
gp->member = member;
gp->score = score;
ga->used++; ga->used++;
return gp; return gp;
} }
...@@ -210,33 +216,33 @@ void addReplyDoubleDistance(client *c, double d) { ...@@ -210,33 +216,33 @@ void addReplyDoubleDistance(client *c, double d) {
} }
/* Helper function for geoGetPointsInRange(): given a sorted set score /* Helper function for geoGetPointsInRange(): given a sorted set score
* representing a point, and a GeoShape, appends this entry as a geoPoint * representing a point, and a GeoShape, checks if the point is within the search area.
* into the specified geoArray only if the point is within the search area.
* *
* returns C_OK if the point is included, or C_ERR if it is outside. */ * shape: the rectangle
int geoAppendIfWithinShape(geoArray *ga, GeoShape *shape, double score, sds member) { * score: the encoded version of lat,long
double distance = 0, xy[2]; * xy: output variable, the decoded lat,long
* distance: output variable, the distance between the center of the shape and the point
*
* Return values:
*
* The return value is C_OK if the point is within search area, or C_ERR if it is outside.
* "*xy" is populated with the decoded lat,long.
* "*distance" is populated with the distance between the center of the shape and the point.
*/
int geoWithinShape(GeoShape *shape, double score, double *xy, double *distance) {
if (!decodeGeohash(score,xy)) return C_ERR; /* Can't decode. */ if (!decodeGeohash(score,xy)) return C_ERR; /* Can't decode. */
/* Note that geohashGetDistanceIfInRadiusWGS84() takes arguments in /* Note that geohashGetDistanceIfInRadiusWGS84() takes arguments in
* reverse order: longitude first, latitude later. */ * reverse order: longitude first, latitude later. */
if (shape->type == CIRCULAR_TYPE) { if (shape->type == CIRCULAR_TYPE) {
if (!geohashGetDistanceIfInRadiusWGS84(shape->xy[0], shape->xy[1], xy[0], xy[1], if (!geohashGetDistanceIfInRadiusWGS84(shape->xy[0], shape->xy[1], xy[0], xy[1],
shape->t.radius*shape->conversion, &distance)) return C_ERR; shape->t.radius*shape->conversion, distance))
return C_ERR;
} else if (shape->type == RECTANGLE_TYPE) { } else if (shape->type == RECTANGLE_TYPE) {
if (!geohashGetDistanceIfInRectangle(shape->t.r.width * shape->conversion, if (!geohashGetDistanceIfInRectangle(shape->t.r.width * shape->conversion,
shape->t.r.height * shape->conversion, shape->t.r.height * shape->conversion,
shape->xy[0], shape->xy[1], xy[0], xy[1], &distance)) shape->xy[0], shape->xy[1], xy[0], xy[1], distance))
return C_ERR; return C_ERR;
} }
/* Append the new element. */
geoPoint *gp = geoArrayAppend(ga);
gp->longitude = xy[0];
gp->latitude = xy[1];
gp->dist = distance;
gp->member = member;
gp->score = score;
return C_OK; return C_OK;
} }
...@@ -257,8 +263,6 @@ int geoGetPointsInRange(robj *zobj, double min, double max, GeoShape *shape, geo ...@@ -257,8 +263,6 @@ int geoGetPointsInRange(robj *zobj, double min, double max, GeoShape *shape, geo
/* That's: min <= val < max */ /* That's: min <= val < max */
zrangespec range = { .min = min, .max = max, .minex = 0, .maxex = 1 }; zrangespec range = { .min = min, .max = max, .minex = 0, .maxex = 1 };
size_t origincount = ga->used; size_t origincount = ga->used;
sds member;
if (zobj->encoding == OBJ_ENCODING_LISTPACK) { if (zobj->encoding == OBJ_ENCODING_LISTPACK) {
unsigned char *zl = zobj->ptr; unsigned char *zl = zobj->ptr;
unsigned char *eptr, *sptr; unsigned char *eptr, *sptr;
...@@ -274,6 +278,8 @@ int geoGetPointsInRange(robj *zobj, double min, double max, GeoShape *shape, geo ...@@ -274,6 +278,8 @@ int geoGetPointsInRange(robj *zobj, double min, double max, GeoShape *shape, geo
sptr = lpNext(zl, eptr); sptr = lpNext(zl, eptr);
while (eptr) { while (eptr) {
double xy[2];
double distance = 0;
score = zzlGetScore(sptr); score = zzlGetScore(sptr);
/* If we fell out of range, break. */ /* If we fell out of range, break. */
...@@ -281,10 +287,11 @@ int geoGetPointsInRange(robj *zobj, double min, double max, GeoShape *shape, geo ...@@ -281,10 +287,11 @@ int geoGetPointsInRange(robj *zobj, double min, double max, GeoShape *shape, geo
break; break;
vstr = lpGetValue(eptr, &vlen, &vlong); vstr = lpGetValue(eptr, &vlen, &vlong);
member = (vstr == NULL) ? sdsfromlonglong(vlong) : if (geoWithinShape(shape, score, xy, &distance) == C_OK) {
sdsnewlen(vstr,vlen); /* Append the new element. */
if (geoAppendIfWithinShape(ga,shape,score,member) char *member = (vstr == NULL) ? sdsfromlonglong(vlong) : sdsnewlen(vstr, vlen);
== C_ERR) sdsfree(member); geoArrayAppend(ga, xy, distance, score, member);
}
if (ga->used && limit && ga->used >= limit) break; if (ga->used && limit && ga->used >= limit) break;
zzlNext(zl, &eptr, &sptr); zzlNext(zl, &eptr, &sptr);
} }
...@@ -299,14 +306,15 @@ int geoGetPointsInRange(robj *zobj, double min, double max, GeoShape *shape, geo ...@@ -299,14 +306,15 @@ int geoGetPointsInRange(robj *zobj, double min, double max, GeoShape *shape, geo
} }
while (ln) { while (ln) {
sds ele = ln->ele; double xy[2];
double distance = 0;
/* Abort when the node is no longer in range. */ /* Abort when the node is no longer in range. */
if (!zslValueLteMax(ln->score, &range)) if (!zslValueLteMax(ln->score, &range))
break; break;
if (geoWithinShape(shape, ln->score, xy, &distance) == C_OK) {
ele = sdsdup(ele); /* Append the new element. */
if (geoAppendIfWithinShape(ga,shape,ln->score,ele) geoArrayAppend(ga, xy, distance, ln->score, sdsdup(ln->ele));
== C_ERR) sdsfree(ele); }
if (ga->used && limit && ga->used >= limit) break; if (ga->used && limit && ga->used >= limit) break;
ln = ln->level[0].forward; ln = ln->level[0].forward;
} }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment