Repository: boto/botocore
Pull request #540 (2015-05-22 23:16:38)

Improve repeated method calls in botocore
Speeds up botocore quite a bit when making repeated calls
to the same operation, e.g. dynamodb.get_item().

The main area of improvement was due to the fact that the operation models were not being cached so repeated calls to `get_item` would result in the op model being recreated each call.  This meant that even though the operation model itself would cache things like the input/output shape, that had no effect because we were throwing away the operation model afterwards.

The other thing that profiling showed was that the pformat() call was slowing things down.  This is still useful information so rather than `pformat()` we're just logging the header dict directly.

This gives performance that's on par with boto2.

cc @kyleknap


Patch:
diff --git a/botocore/model.py b/botocore/model.py
index 20e4f7faa..0fc1913b8 100644
--- a/botocore/model.py
+++ b/botocore/model.py
@@ -1,7 +1,19 @@
+# Copyright 2015 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
 """Abstractions to interact with service models."""
 from collections import defaultdict
 
-from botocore.utils import CachedProperty
+from botocore.utils import CachedProperty, instance_cache
 from botocore.compat import OrderedDict
 
 
@@ -220,6 +232,7 @@ class ServiceModel(object):
             service_description.get('shapes', {}))
         self._signature_version = NOT_SET
         self._service_name = service_name
+        self._instance_cache = {}
 
     def shape_for(self, shape_name, member_traits=None):
         return self._shape_resolver.get_shape_by_name(
@@ -228,6 +241,7 @@ class ServiceModel(object):
     def resolve_shape_ref(self, shape_ref):
         return self._shape_resolver.resolve_shape_ref(shape_ref)
 
+    @instance_cache
     def operation_model(self, operation_name):
         try:
             model = self._service_description['operations'][operation_name]
@@ -433,7 +447,8 @@ class ShapeResolver(object):
         if member_traits:
             shape_model = shape_model.copy()
             shape_model.update(member_traits)
-        return shape_cls(shape_name, shape_model, self)
+        result = shape_cls(shape_name, shape_model, self)
+        return result
 
     def resolve_shape_ref(self, shape_ref):
         # A shape_ref is a dict that has a 'shape' key that
diff --git a/botocore/parsers.py b/botocore/parsers.py
index 57d499fa7..9e0b88554 100644
--- a/botocore/parsers.py
+++ b/botocore/parsers.py
@@ -95,7 +95,6 @@ import base64
 import json
 import xml.etree.cElementTree
 import logging
-from pprint import pformat
 
 from botocore.compat import six, XMLParseError
 
@@ -201,7 +200,7 @@ class ResponseParser(object):
             always be present.
 
         """
-        LOG.debug('Response headers:\n%s', pformat(dict(response['headers'])))
+        LOG.debug('Response headers: %s', response['headers'])
         LOG.debug('Response body:\n%s', response['body'])
         if response['status_code'] >= 301:
             parsed = self._do_error_parse(response, shape)
diff --git a/botocore/utils.py b/botocore/utils.py
index 11d29ca32..745dc0551 100644
--- a/botocore/utils.py
+++ b/botocore/utils.py
@@ -14,8 +14,8 @@ import re
 import logging
 import datetime
 import hashlib
-import math
 import binascii
+import functools
 
 from six import string_types, text_type
 import dateutil.parser
@@ -95,8 +95,8 @@ def remove_dot_segments(url):
 
 
 def validate_jmespath_for_set(expression):
-    # Validates a limited jmespath expression to determine if we can set a value
-    # based on it. Only works with dotted paths.
+    # Validates a limited jmespath expression to determine if we can set a
+    # value based on it. Only works with dotted paths.
     if not expression or expression == '.':
         raise InvalidExpressionError(expression=expression)
 
@@ -121,10 +121,10 @@ def set_value_from_jmespath(source, expression, value, is_first=True):
         raise InvalidExpressionError(expression=expression)
 
     if remainder:
-        if not current_key in source:
+        if current_key not in source:
             # We've got something in the expression that's not present in the
-            # source (new key). If there's any more bits, we'll set the key with
-            # an empty dictionary.
+            # source (new key). If there's any more bits, we'll set the key
+            # with an empty dictionary.
             source[current_key] = {}
 
         return set_value_from_jmespath(
@@ -223,7 +223,7 @@ def parse_key_val_file(filename, _open=open):
         with _open(filename) as f:
             contents = f.read()
             return parse_key_val_file_contents(contents)
-    except OSError as e:
+    except OSError:
         raise ConfigNotFound(path=filename)
 
 
@@ -644,3 +644,35 @@ def _is_get_bucket_location_request(request):
 
 def _allowed_region(region_name):
     return region_name not in RESTRICTED_REGIONS
+
+
+def instance_cache(func):
+    """Method decorator for caching method calls to a single instance.
+
+    **This is not a general purpose caching decorator.**
+
+    In order to use this, you *must* provide an ``_instance_cache``
+    attribute on the instance.
+
+    This decorator is used to cache method calls.  The cache is only
+    scoped to a single instance though such that multiple instances
+    will maintain their own cache.  In order to keep things simple,
+    this decorator requires that you provide an ``_instance_cache``
+    attribute on your instance.
+
+    """
+    func_name = func.__name__
+
+    @functools.wraps(func)
+    def _cache_guard(self, *args, **kwargs):
+        cache_key = (func_name, args)
+        if kwargs:
+            kwarg_items = tuple(sorted(kwargs.items()))
+            cache_key = (func_name, args, kwarg_items)
+        result = self._instance_cache.get(cache_key)
+        if result is not None:
+            return result
+        result = func(self, *args, **kwargs)
+        self._instance_cache[cache_key] = result
+        return result
+    return _cache_guard


Test patch:
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index 4320dfdf7..434d9905b 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -35,6 +35,7 @@ from botocore.utils import calculate_tree_hash
 from botocore.utils import calculate_sha256
 from botocore.utils import is_valid_endpoint_url
 from botocore.utils import fix_s3_host
+from botocore.utils import instance_cache
 from botocore.model import DenormalizedStructureBuilder
 from botocore.model import ShapeResolver
 
@@ -550,5 +551,48 @@ class TestFixS3Host(unittest.TestCase):
         self.assertEqual(request.url, original_url)
 
 
+class TestInstanceCache(unittest.TestCase):
+    class DummyClass(object):
+        def __init__(self, cache):
+            self._instance_cache = cache
+
+        @instance_cache
+        def add(self, x, y):
+            return x + y
+
+        @instance_cache
+        def sub(self, x, y):
+            return x - y
+
+    def setUp(self):
+        self.cache = {}
+
+    def test_cache_single_method_call(self):
+        adder = self.DummyClass(self.cache)
+        self.assertEqual(adder.add(2, 1), 3)
+        # This should result in one entry in the cache.
+        self.assertEqual(len(self.cache), 1)
+        # When we call the method with the same args,
+        # we should reuse the same entry in the cache.
+        self.assertEqual(adder.add(2, 1), 3)
+        self.assertEqual(len(self.cache), 1)
+
+    def test_can_cache_multiple_methods(self):
+        adder = self.DummyClass(self.cache)
+        adder.add(2, 1)
+
+        # A different method results in a new cache entry,
+        # so now there should be two elements in the cache.
+        self.assertEqual(adder.sub(2, 1), 1)
+        self.assertEqual(len(self.cache), 2)
+        self.assertEqual(adder.sub(2, 1), 1)
+
+    def test_can_cache_kwargs(self):
+        adder = self.DummyClass(self.cache)
+        adder.add(x=2, y=1)
+        self.assertEqual(adder.add(x=2, y=1), 3)
+        self.assertEqual(len(self.cache), 1)
+
+
 if __name__ == '__main__':
     unittest.main()