For the past week or so, there seems to have been an influx of new bugs reported on Bugzilla that centre around making the Marketplace perform better under its high traffic load. The codebase is becoming increasingly more stable on a daily basis but ensuring that the website maintains a snappy feel brings up its own set of problems. I’ve always had an interest in scaling and efficiency so this week I have tackled two bugs that should hopefully assist in this area.
Optimizing App IconsWhile scanning the list of potential bugs, I came across one that seemed interesting. It detailed that whenever an application was uploaded to the marketplace, we should be losslessly optimizing the app’s icon so that we are not required to serve the larger files. This bug was particularly appealing to me since it mentioned the use of a utility called Pngcrush, which I had heard about before when I was implementing an asset management system at my previous co-op placement. I contacted the developer that suggested the bug to ensure that it would be ok if I worked on it and to get his opinion on my proposed implementation. He seemed content with me working on it but mentioned that I would have to act quickly since it could become a higher priority (at which point he would steal the bug back from me). This seemed reasonable so I got to work right away.
My first step in solving this bug was to install the Pngcrush utility. Using
brew on Mac OSX this is quite simple:
brew install pngcrush
I took a look through the existing code and identified a task, which was used to resize the icons. I fired up my development server and submitted a new test app to ensure that the
resize_icon task was being called. From this task, I was able to identify where the icons were being saved to locally (in a nested structure within the tmp directory in the project). I opted to manually run Pngcrush on some of the test icons to gauge what the results would actually be like once my new code addition was working properly. I found that the test icon that I was using was reduced from 208 bytes to 135 bytes.
I added a new celery task called
pngcrush_icon, which would be run asynchronously from the
resize_icon task. The task works by opening a subprocess and calling the Pngcrush utility with appropriate arguments and the path to the existing icon file. One design decision that I made was that the original icon file would be overwritten with the new optimized version. There didn’t seem to be a direct downside to doing this and I figured that it would ensure compatibility across the project.
With the task in place, I performed a test run but found that my code was not running at all. I quickly realized that the
resize_icon task was swallowing all exceptions and I was invoking the
pngcrush_icon task with incorrect arguments. After making that change, the task was running and optimizing each icon, at all sizes! In the development environment, tasks are run synchronously so I also had to ensure that the task would run when it was setup with the task queue. I modified my settings and started up the Celery workers and everything seemed to continue working as expected.
At this point, I turned my attention to writing some tests for my added functionality. One of the tests that I added was to ensure that the optimized icon was actually smaller than the original icon. I added a second test to ensure that the optimized icon was still a valid image after Pngcrush had been run. I opened a new pull request for my addition. I have received a couple of pieces of feedback on this pull request so far. I was asked to change one of my tests in order to verify that the crushed image was still the same as the other image (since it is a lossless optimization after all). I was pointed to a library function that would allow me to take the difference between two images and determine if there were any resulting pixel changes. I was also asked to add some extra error checking and to update the documentation so I did that as well in a new commit.
Removing Unused API Response DataWith that last bug fix complete, I started in on a new bug which involved the removal of some of the unused icon URLs in an API response. As is, the API response returns icon URLs for four different icon sizes but only one is used on the Marketplace. I added some featured apps in my local environment so that they would show up in the API response first. I was able to make use of the Curation tool that I mentioned in my previous blog post.
Next, I had to figure out how the API response was actually being generated. After some searching, I found that each API view had an associated serializer. This serializer would determine which fields should be displayed and how the field’s content would be retrieved. With this knowledge, I was able to override the
get_icons method in the specific serializer that I wanted to modify. My new
get_icons method would only return a single element dictionary with the corresponding icon URL (rather than a four element dictionary with all icon URLs). After some manual verification that the new API response looked accurate, I ran the existing test suite for the Fireplace API methods:
make SETTINGS=settings_local_mkt ARGS='--verbosity 2 zamboni.mkt.fireplace.tests.test_api' test
All tests were passing which meant I hadn’t broken anything (yet, at least) so I added a new unit test in order to confirm that the content that was returned for the icons portion of the API response only consisted of a single URL for the 64px by 64px icon. I had to perform some normalization on the URLs in my test using the
urlparse module since the test results were coming back with different modification dates in the query string. Once this was complete, I opened up a new pull request on the Marketplace project. I have received some great feedback on this pull request, which mentioned that I should do some refactoring to use my change as part of one of the other serializers as well. You can see my attempt to address this feedback in this commit.
All in all, I’ve had another fantastic week working on the Mozilla Marketplace project and I have already scoped out some new bugs that I will be squashing in the coming days!