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
- Any requests that aren't a
GET
will trigger a 500 error.
- Your logic tends to repeat iteself when it doesn't need to.
- 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:
- Filter the
FloodHazard
objects by the specific type.
- Get all of the ids of the filtered objects.
- You go through the list of ids, one-by-one, and do the following:
- Retrieve the
FloodHazard
object for the id.
- Get all of the
PolyStructure
objects which intersect with it.
- Add the ids of those
PolyStructure
objects to a master list of PolyStructure
ids for the type.
- 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
- 3 queries are the beginning for getting the list of
FloodHazard
objects for each type.
- Note that this retrieved ids for n objects.
- For each
FloodHazard
object you are
- Getting the same
FloodHazard
object (n extra queries, n + 3 so far)
- Getting the intersecting
PolyStructure
objects (n extra queries, 2n + 3 so far)
- 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)
pprofile
orline_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\$