[PATCH aeolus-image-rubygem] RFC - lazy warehouse attributes loading

jprovazn at redhat.com jprovazn at redhat.com
Wed Feb 8 09:51:45 UTC 2012


From: Jan Provaznik <jprovazn at redhat.com>

this patch is not supposed to be pushed, it's just an experimental
version. As I mentioned in some mails before, I think that one bottle
neck of warehouse lib is autoloading of all object attributes when getting
a warehouse object. The idea of this patch is to load attributes on demand.

Second optimization is skipping of uuid attribute loading -
uuid is used as a key when saving an object in buckets, IOW
key == uuid, e.g.:
$ curl http://localhost:9090/builds/7c5d70ff-b998-4bb1-8c9a-b8cf3d5dd2a2/uuid
7c5d70ff-b998-4bb1-8c9a-b8cf3d5dd2a2

Generally it's possible to have key != uuid from warehouse point of view,
but both imagefactory and conductor use uuid as key, so this optimization
can be used now.

Here are some numbers (times are from firebug, client side), measured on a
deployable with 25 identical images:
before patch:
deployable show page: 19s (1600 requests)
launch deployment: 51s (2300 requests)
image show page: 4.5s (47 requests) (1 provider image)
image index page: 6.3s (122 requests) (5 images)

lazyload + uuid (deployable show page):
deployable show page: 10s (750 requests)
launch deployment: 43s (1125 requests)
image show page: 3.5s (19 requests) (1 provider image)
image index page: 5.5s (78 requests) (5 images)

(Honestly I expected better improvement).

Do we want to get this in for 1.0 release? (For single image deployments there
won't be big difference).
---
 lib/aeolus_image/model/warehouse/icicle.rb         |    2 +-
 lib/aeolus_image/model/warehouse/image.rb          |   12 +++++-----
 lib/aeolus_image/model/warehouse/image_build.rb    |    6 ++--
 lib/aeolus_image/model/warehouse/provider_image.rb |    6 ++--
 lib/aeolus_image/model/warehouse/target_image.rb   |   10 ++++----
 .../model/warehouse/warehouse_model.rb             |   24 ++++++++++++++++---
 6 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/lib/aeolus_image/model/warehouse/icicle.rb b/lib/aeolus_image/model/warehouse/icicle.rb
index 1a11da8..a1cfa96 100644
--- a/lib/aeolus_image/model/warehouse/icicle.rb
+++ b/lib/aeolus_image/model/warehouse/icicle.rb
@@ -43,7 +43,7 @@ module Aeolus
 
         def get_icicle
           unless @icicle_xml
-              icicle = Icicle.bucket.objects.find(@uuid) if @uuid
+              icicle = Icicle.bucket.objects.find(uuid) if uuid
             begin
               @icicle_xml = Nokogiri::XML icicle.first.body
             rescue
diff --git a/lib/aeolus_image/model/warehouse/image.rb b/lib/aeolus_image/model/warehouse/image.rb
index 7a06f0e..c48ae20 100644
--- a/lib/aeolus_image/model/warehouse/image.rb
+++ b/lib/aeolus_image/model/warehouse/image.rb
@@ -43,12 +43,12 @@ module Aeolus
         end
 
         def latest_pushed_build
-          ImageBuild.find(@latest_build) if @latest_build
+          ImageBuild.find(latest_build) if respond_to?(:latest_build)
         end
 
         def latest_pushed_or_unpushed_build
-          build = @latest_build ? ImageBuild.find(@latest_build) : nil
-          push = @latest_unpushed ? ImageBuild.find(@latest_unpushed) : nil
+          build = respond_to?(:latest_build) ? ImageBuild.find(latest_build) : nil
+          push = respond_to?(:latest_unpushed) ? ImageBuild.find(latest_unpushed) : nil
           if build and push
             return push.timestamp > build.timestamp ? push : build
           end
@@ -56,7 +56,7 @@ module Aeolus
         end
 
         def image_builds
-          ImageBuild.where("($image == \"" + @uuid.to_s + "\")")
+          ImageBuild.where("($image == \"" + uuid.to_s + "\")")
         end
 
         # Return all Provider Images associated with this Image
@@ -101,12 +101,12 @@ module Aeolus
             end
           rescue NoMethodError
           end
-          Image.delete(@uuid)
+          Image.delete(uuid)
         end
 
         # No template is created on imported images
         def imported?
-          !@template
+          !respond_to?(:template)
         end
 
         # template_xml.xpath(PATH).text => template_xpath(PATH)
diff --git a/lib/aeolus_image/model/warehouse/image_build.rb b/lib/aeolus_image/model/warehouse/image_build.rb
index 5b9f7d4..8f5dcf5 100644
--- a/lib/aeolus_image/model/warehouse/image_build.rb
+++ b/lib/aeolus_image/model/warehouse/image_build.rb
@@ -19,11 +19,11 @@ module Aeolus
         @bucket_name = 'builds'
 
         def image
-          Image.find(@image) if @image
+          Image.find(_image) if respond_to?(:_image)
         end
 
         def target_images
-          TargetImage.where("($build == \"" + @uuid.to_s + "\")")
+          TargetImage.where("($build == \"" + uuid.to_s + "\")")
         end
 
         # Convenience Method to get all provider images for this build
@@ -52,7 +52,7 @@ module Aeolus
             end
           rescue NoMethodError
           end
-          ImageBuild.delete(@uuid)
+          ImageBuild.delete(uuid)
         end
       end
     end
diff --git a/lib/aeolus_image/model/warehouse/provider_image.rb b/lib/aeolus_image/model/warehouse/provider_image.rb
index 6a65ab7..83c7a2f 100644
--- a/lib/aeolus_image/model/warehouse/provider_image.rb
+++ b/lib/aeolus_image/model/warehouse/provider_image.rb
@@ -19,16 +19,16 @@ module Aeolus
         @bucket_name = 'provider_images'
 
         def target_image
-          TargetImage.find(@target_image) if @target_image
+          TargetImage.find(target_image) if respond_to?(:target_image)
         end
 
         def provider_name
-          @provider
+          provider
         end
 
         # Deletes this provider image
         def delete!
-          ProviderImage.delete(@uuid)
+          ProviderImage.delete(uuid)
         end
       end
     end
diff --git a/lib/aeolus_image/model/warehouse/target_image.rb b/lib/aeolus_image/model/warehouse/target_image.rb
index 5e6b84c..e3e88af 100644
--- a/lib/aeolus_image/model/warehouse/target_image.rb
+++ b/lib/aeolus_image/model/warehouse/target_image.rb
@@ -19,15 +19,15 @@ module Aeolus
         @bucket_name = 'target_images'
 
         def build
-          ImageBuild.find(@build) if @build
+          ImageBuild.find(_build) if respond_to?(:_build)
         end
 
         def provider_images
-          ProviderImage.where("($target_image == \"" + @uuid.to_s + "\")")
+          ProviderImage.where("($target_image == \"" + uuid.to_s + "\")")
         end
 
         def target_template
-          Template.find(@template) if @template
+          Template.find(template) if respond_to?(:template)
         end
 
         # Deletes this targetimage and all child objects
@@ -38,11 +38,11 @@ module Aeolus
             end
           rescue NoMethodError
           end
-          TargetImage.delete(@uuid)
+          TargetImage.delete(uuid)
         end
 
         def find_provider_image_by_provider_and_account(provider, provider_account)
-          conditions = ["$target_image == \"#{@uuid}\""]
+          conditions = ["$target_image == \"#{uuid}\""]
           conditions << "$provider == \"#{provider}\"" if provider
           conditions << "$provider_account_identifier == \"#{provider_account}\"" if provider_account
           ProviderImage.where('(' + conditions.join(' && ') + ')')
diff --git a/lib/aeolus_image/model/warehouse/warehouse_model.rb b/lib/aeolus_image/model/warehouse/warehouse_model.rb
index dd494d6..ca53fa4 100644
--- a/lib/aeolus_image/model/warehouse/warehouse_model.rb
+++ b/lib/aeolus_image/model/warehouse/warehouse_model.rb
@@ -20,14 +20,30 @@ module Aeolus
 
       class WarehouseModel
         attr_writer :body
+        attr_reader :obj
+        attr_accessor :uuid
 
         def initialize(obj)
           @obj = obj
-          @attrs = obj.attrs(obj.attr_list)
-          @attrs.each do |k,v|
+          @uuid = @obj.key
+          @attr_list = obj.attr_list
+          @attr_list.each do |k|
             self.class.send(:attr_writer, k.to_sym) unless respond_to?(:"#{k}=")
-            self.class.send(:attr_reader, k.to_sym) unless respond_to?(k.to_sym)
-            send(:"#{k}=", v)
+            lazy_attr_reader(k)
+          end
+        end
+
+        def lazy_attr_reader(orig_attr)
+          attr = respond_to?(orig_attr.to_sym) ? "_#{orig_attr}" : orig_attr
+          # TODO: security - restrict attr names
+          attr_sym = "@#{attr}".to_sym
+          (class << self; self; end).class_eval do
+            define_method attr do
+              unless instance_variable_defined?(attr_sym)
+                instance_variable_set(attr_sym, @obj.attr(orig_attr))
+              end
+              instance_variable_get(attr_sym)
+            end
           end
         end
 
-- 
1.7.7.5




More information about the aeolus-devel mailing list