1
\$\begingroup\$

Below is my query to identify what hazard type does the buildings belong:

def floodhazard_tbl(request):
    if request.method == "GET":
        reference_high = FloodHazard.objects.filter(hazard='High')
        reference_medium = FloodHazard.objects.filter(hazard='Medium')
        reference_low = FloodHazard.objects.filter(hazard='Low')

        #get all ids based on filter
        ids_high = reference_high.values_list('id', flat=True)
        ids_medium = reference_medium.values_list('id', flat=True)
        ids_low = reference_low.values_list('id', flat=True)

        # create a list
        k = []
        response_high = []
        response_medium = []
        response_low = []
        bldg_id_high = []
        bldg_ids_high = []
        bldg_id_medium = []
        bldg_ids_medium = []
        filtered_bldg = []

        # this code is results a messy JSON data that need underscore.js to manipulate
        # in order for us to use datatables
        for myid in ids_high:
            getgeom = FloodHazard.objects.get(id=myid).geom
            response_high = PolyStructures.objects.filter(geom__intersects=getgeom).values(
                'brgy_locat', 'municipali').annotate(counthigh=Count('brgy_locat'))
            bldg_id_high = filter(None,response_high.values_list('id',flat=True))
            bldg_ids_high = bldg_ids_high + bldg_id_high
            k.append(response_high)


        for myid in ids_medium:
            getgeom = FloodHazard.objects.get(id=myid).geom
            response_medium = PolyStructures.objects.exclude(id__in=bldg_ids_high).filter(geom__intersects=getgeom).values(
                'brgy_locat', 'municipali').annotate(countmedium=Count('brgy_locat'))
            bldg_id_medium = filter(None,response_medium.values_list('id',flat=True))
            bldg_ids_medium = bldg_ids_medium + bldg_id_medium
            k.append(response_medium)

        filtered_bldg = bldg_ids_medium + bldg_ids_high

        for myid in ids_low:
            getgeom = FloodHazard.objects.get(id=myid).geom
            response_low = PolyStructures.objects.exclude(id__in=filtered_bldg).filter(geom__intersects=getgeom).values(
                'brgy_locat', 'municipali').annotate(countlow=Count('brgy_locat'))
            k.append(response_low)


        #to_json = list(k for k,_ in itertools.groupby(k))
        result = []
        for d in chain.from_iterable(k):
            result.append(d)

        return HttpResponse(list(json.dumps(result)), content_type='application/json')

I have this method (algo):

After the response_high query, I get all the building that DID NOT intersect to the HIGH type of hazard and so on and exclude it in the next query (intersects) to avoid duplication (because there is a tendency that the building will intersect to the hazard polygon several times).

The code works but the response is slow, take 6 seconds to show the output. As it is messed up, any optimization "technique" is appreciated.

\$\endgroup\$
3
  • \$\begingroup\$ Can you run this under pprofile or line_profiler? This will help find what things need to be fixed to get speed. I think you'll also be short on answers because we probably need to know GeoDjango to help much. \$\endgroup\$
    – Veedrac
    Commented Jun 6, 2015 at 2:29
  • \$\begingroup\$ In addition to needing to know GeoDjango, you are also forcing us to guess what your models are set up like. It's most likely your queries that are slow (I see a few N+1 queries), so the models would be needed to optimise them. \$\endgroup\$ Commented Jun 6, 2015 at 4:02
  • \$\begingroup\$ Models(FloodHazard, PolyStructures) are non-related, no relationship at all. \$\endgroup\$ Commented Jun 6, 2015 at 12:21

1 Answer 1

2
\$\begingroup\$

I don't have a lot of experience with GeoDjango specifically, so there are some parts here that I can't review, but there are some spots that stand out as needing improvement.

The performance issues in your view

  1. Any requests that aren't a GET will trigger a 500 error.
  2. Your logic tends to repeat iteself when it doesn't need to.
  3. You are triggering a ton of queries to generate the response.

Non-GET requests trigger a 500 error

Your floodhazard_tbl view specifically checks for a GET request and then returns the list of results based on that, but it doesn't actually return anything if a different request type is sent. Because of this, Django will trigger an exception (because no response was returned) which will trigger a 500 error for any non-GET response. You should instead return a HttpResponseNotAllowed response for any non-GET request, which is the equivalent of a 405 response.

Walking through how the response is generated

Right now you are going through the follow steps to get the buildings that intersect a specific type of hazard:

  1. Filter the FloodHazard objects by the specific type.
  2. Get all of the ids of the filtered objects.
  3. You go through the list of ids, one-by-one, and do the following:
    1. Retrieve the FloodHazard object for the id.
    2. Get all of the PolyStructure objects which intersect with it.
    3. Add the ids of those PolyStructure objects to a master list of PolyStructure ids for the type.
    4. Add a serialized version of these PolyStructure objects to a master list of results.

You'll notice that steps 2 and 3.1 can cancel themselves out. And because you are getting the FloodHazard object again, you are causing a N+1 query at 3.1 because the object has not been cached. There is also a N+1 query happening at 3.2 when you get the PolyStructure objects.

The database queries that are triggered

Now, you have multiple N+1 queries happening in the body of your view that is likely leading to the increased response time. When working with a large number of objects this can be the slowest part of your response, as each query adds a small amount of overhead that can quickly build up into a 5+ second response.

So, to give you a rough idea of how many queries are being executed here, lets take a count

  1. 3 queries are the beginning for getting the list of FloodHazard objects for each type.
    • Note that this retrieved ids for n objects.
  2. For each FloodHazard object you are
    1. Getting the same FloodHazard object (n extra queries, n + 3 so far)
    2. Getting the intersecting PolyStructure objects (n extra queries, 2n + 3 so far)
    3. Getting the ids of the PolyStructure objects (n * (2/3) extra queries, one for each type excluding "Low")

So while the 2.3 doesn't trigger an extra n queries, depending on the number of "Low" FloodHazard objects there could still be a significant number of queries triggered.

After adding this all up, you will see that roughly 3 queries are triggered for each FloodHazard object in the original query. So if there were 100 "High" FloodHazard objects, you would be triggering 303 queries to generate the response.

How things can be improved

There are four easy fixes that can be made to significantly drop the number of queries made, which will end up improving performance in the long run.

Fixing the 500 error on non-GET requests

This one is the easiest fix, as it just requires you to return a HttpResponseNotAllowed at the end of your method.

def floodhazard_tbl(request):
    from django.http import HttpResponseNotAllowed

    if request.method == "GET":
        # We'll get to this
        pass

    return HttpResponseNotAllowed(['GET'])

So now if anyone makes a POST (or anything other than GET) request, they will get a 405 response with the Allow header set to GET.

Cutting out the duplicate FloodHazard queries

Right now you are retrieving a list of ids of FloodHazard filtered down to specific types, and then you are going through the list of ids and making a query to get the FloodHazard object back. Since you don't actually do anything else with the ids of these FloodHazard objects, you can skip the call to values_list and just use the original query when iterating over the list of objects.

high_hazards = FloodHazard.objects.filter(hazard='High')
medium_hazards = FloodHazard.objects.filter(hazard='Medium')
low_hazards = FloodHazard.objects.filter(hazard='Low')

for hazard in high_hazards:
    pass

for hazard in medium_hazards:
    pass

for hazard in low_hazards:
    pass

So now instead of re-querying the FloodHazard objects, you will have it available as hazard in each iteration. You can still get the geometry as hazard.geom, and you cut out n queries without changing the behaviour of the view.

Now you're down from 303 queries to 203 queries, just by making the one simple change.

Getting the areas covered by the FloodHazard objects

Right now you are getting the PolyStructure objects for each FloodHazard area one-by-one, in an attempt to get all PolyStructures objects that fall within the area of the FloodHazard objects. You can improve the performance here by getting the union of all of the FloodHazard areas, and then getting all PolyStructure objects within that combined area.

GeoDjango provides a Union aggregate as well as a Collect aggregate, with the main difference between them being that Union will remove any overlapping boundaries while Collect will just combine them all together. You are going to need to determine which one works best for your dataset in the long run.

high_hazards = FloodHazard.objects.filter(hazard='High').aggregate(combined_geom=Collect('geom'))
medium_hazards = FloodHazard.objects.filter(hazard='Medium').aggregate(combined_geom=Collect('geom'))
low_hazards = FloodHazard.objects.filter(hazard='Low').aggregate(combined_geom=Collect('geom'))

high_area = high_hazards['combined_geom']
medium_area = medium_hazards['combined_geom']
low_area = low_hazards['combined_geom']

This will combine the geom field for each object while retrieving the list of objects and place it in the combined_geom index of the queryset that is returned. I've split each of these out into their own *_area variables, which will contain the GEOSGeometry or GeometryCollection object representing the combined geometry fields.

Retreiving all of the PolyStructures objects for a set of FloodHazard objects

With the object returned by Union or Collect, you can use it in most geospatial queries, including the __intersects filter on geometry fields. So you would still use your existing PolyStructures query but instead of passing getgeom you would pass the *_areas object instead.

high_structures = PolyStructures.objects.filter(geom__intersects=high_area)

So now instead of making n queries to get the PolyStructures objects within each FloodHazard area, you are just making a single query that gets them all at once.

Now you're down from 203 queries to 106 queries (one new query for each type), but the logic within your views has changed as a result.

Getting the ids of the PolyStructures objects so they are excluded from other types

In order to get the ids of the structures, you have two different options: use the values_list('id', flat=True) method like you already are, or use map to retrieve the id field from each of the objects in the queryset we used before. Most likely the values_list option is going to be faster, but it's useful to note that it's not always the only option.

high_ids = high_structures.values_list('id', flat=True)
medium_ids = high_structures.values_list('id', flat=True)

Now instead of these ids being retrieved and grouped on an object-by-object basis, these are retrieved all at once with a single query.

Now you're down from 106 queries to 8 queries, which all comes together to be a significant decrease.

Putting it all together

So now if you combine all of the suggestions into a single view, you get 34 lines of code (including comments and whitespace) that executes 8 queries to the database and returns the same response.

# There are three different types of hazards, we are going to get the combined geometry for each type
high_hazards = FloodHazard.objects.filter(hazard='High').aggregate(combined_geom=Collect('geom'))
medium_hazards = FloodHazard.objects.filter(hazard='Medium').aggregate(combined_geom=Collect('geom'))
low_hazards = FloodHazard.objects.filter(hazard='Low').aggregate(combined_geom=Collect('geom'))

# Get the areas from the query
high_area = high_hazards['combined_geom']
medium_area = medium_hazards['combined_geom']
low_area = low_hazards['combined_geom']

# Get all structures in each of the areas

high_structures = PolyStructures.objects.filter(geom__intersects=high_area)
high_ids = high_structures.values_list('id', flat=True)

medium_structures = PolyStructures.objects.exclude(id__in=high_ids).filter(geom__intersects=medium_area)
medium_ids = medium_structures.values_list('id', flat=True)

combined_ids = list(high_ids) + list(medium_ids)

low_structures = PolyStructures.objects.exclude(id__in=combined_ids).filter(geom__intersects=low_area)

# Serialize the structures

high_results = high_structures.values('brgy_locat', 'municipali').annotate(counthigh=Count('brgy_locat'))
medium_results = medium_structures.values('brgy_locat', 'municipali').annotate(countmedium=Count('brgy_locat'))
low_results = low_structures.values('brgy_locat', 'municipali').annotate(countlow=Count('brgy_locat'))

# Combine all of the structures into a unified response

results = list(high_results) + list(medium_results) + list(low_results)

# Return all of the results as JSON
return JsonResponse(results, safe=False)
\$\endgroup\$
7
  • \$\begingroup\$ Thank you so much for enlightening me. It's really hard to learn Django when you don't have a background in Python. It returns an error: unsupported operand type(s) for +: 'GeoValuesListQuerySet' and 'GeoValuesListQuerySet' on line combined_ids = high_ids + medium_ids \$\endgroup\$ Commented Jun 8, 2015 at 2:38
  • \$\begingroup\$ I have managed to make this work, using Django Toolbar the query has been reduced to 8 queries but has a significant increase in time which is 17529.00 ms. I'll try your method using much larger shapefile (multipolygon) if its response time is still the same. \$\endgroup\$ Commented Jun 8, 2015 at 5:15
  • \$\begingroup\$ @SachiTekina I suspect you are dealing with a lot more data than I had originally predicted. So I would recommend only following the first two recommendations, as those should apply to all environments. \$\endgroup\$ Commented Jun 8, 2015 at 19:44
  • \$\begingroup\$ The idea of combining(Collect/Union) the geom of each hazard type is great, but does it make the response time faster or slower? I am kinda confused because the query was significantly down to 8, compared to before it was 1k+ but the response time increases. \$\endgroup\$ Commented Jun 9, 2015 at 0:55
  • 1
    \$\begingroup\$ @SachiTekina I would recommend using django-debug-toolbar to compare speeds of the queries, it might be that the combined query is taking considerably longer because there are so many polygons involved. You also might want to check out Union instead of Collect, which might reduce the query size at the cost of some out-of-query data processing. \$\endgroup\$ Commented Jun 9, 2015 at 1:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.