Issue 3750 - Avoid re-generating subscription list for each locale

filters/get_subscriptions.py

 # This file is part of the Adblock Plus website,
 # Copyright (C) 2006-2016 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 import codecs
 import os
 import tarfile
 import urllib
 import logging
 from ConfigParser import SafeConfigParser
+subscriptions = None
 
 def get_subscriptions(_):

[Wladimir Palant, 2016/03/09 11:12:27]

This shouldn't be a filter but rather a global function, then we wouldn't need
this bogus parameter - I think we didn't have globals back when we implemented
this. Could you move this file into globals/ and change the call while we are at
it?

[Sebastian Noack, 2016/03/09 11:18:17]

I agree that this should rather be a global. However, note that caching
templates, as I suggested before, won't automatically cache that function
anymore if it's a global rather than a filter.

We might want to consider though, also caching templates (by a shared
environment), rather than creating a new environment and parse each template for
every page and locale.

[Wladimir Palant, 2016/03/09 11:25:51]

That's fine IMHO, with something like this function it cannot really be decided
automatically that memoization is safe - which is why we should do it
explicitly.
That's something we can look into, I didn't really consider it a priority as
long as the performance is still acceptable - IMHO independent of the issue at
hand.

[Sebastian Noack, 2016/03/09 11:54:18]

Whether you think it's safe or not, jinja's optimizer computes filters and
inlines the result at compile time wherever possible, so whenever the input is
just a constant value and the output can represented as Python literal. However,
since the template is currently parsed again every time, also this filter is
called again. But that optimization doesn't work anyway for globals, as jinja
doesn't know at compile time whether the called function is an environment's
global or whether it's given/overridden by the context at runtime. But I agree,
it's still better to use a global here.
Yes, if we make it a global anyway, it's unrelated.

[Wladimir Palant, 2016/03/09 13:50:15]

This makes sense because for filters same input should always produce the same
output. Which again shows that what we have here is *not* a filter.

+  global subscriptions
+  if not subscriptions is None:
+    return subscriptions
   try:
     from sitescripts.subscriptions import subscriptionParser
   except ImportError:
     logging.warning("Unable to import sitescripts, proceeding with empty "
                     "subscriptions list.")
     return []
 
   result = {}
   utf8_reader = codecs.getreader('utf8')
   source = urllib.urlopen("https://hg.adblockplus.org/subscriptionlist/archive/default.tar.gz")
@@ skipping 16 matching lines @@
         filedata = subscriptionParser.parse_file(fileinfo.name, utf8_reader(archive.extractfile(fileinfo)))
         if filedata.unavailable:
           continue
 
         result[filedata.name] = filedata
   finally:
     source.close()
     subscriptionParser.get_settings = orig_get_settings
 
   subscriptionParser.calculate_supplemented(result)
-  return result.values()
+  subscriptions = result.values()
+  return subscriptions