From 758b62e7d599c58db0f0b29e90e7a588c57a893b Mon Sep 17 00:00:00 2001
From: Matchu <matchu1993@gmail.com>
Date: Mon, 27 May 2024 16:21:22 -0700
Subject: [PATCH] Improve performance of Owls values in Item Getting Guide

Now we preload them all concurrently, instead of in sequence when the
template gets around to asking for them!
---
 app/controllers/items_controller.rb |  6 +++++
 app/models/item.rb                  | 40 ++++++++++++++++++++++++++---
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/app/controllers/items_controller.rb b/app/controllers/items_controller.rb
index eed7fe03..b58d577e 100644
--- a/app/controllers/items_controller.rb
+++ b/app/controllers/items_controller.rb
@@ -128,6 +128,9 @@ class ItemsController < ApplicationController
     @np_items = @items.select(&:np?)
     @pb_items = @items.select(&:pb?)
 
+    # Start loading the NC trade values for the non-Mall NC items.
+    trade_values_task = Async { Item.preload_nc_trade_values(@other_nc_items) }
+
     # Also, PB items have some special handling: we group them by color, then
     # load example pet types for the colors that don't have paint brushes.
     @pb_items_by_color = @pb_items.group_by(&:pb_color).
@@ -144,6 +147,9 @@ class ItemsController < ApplicationController
       [color, color.example_pet_type(preferred_species: species)]
     end.to_h
 
+    # Finish loading the NC trade values.
+    trade_values_task.wait
+
     render layout: "application"
   end
 
diff --git a/app/models/item.rb b/app/models/item.rb
index e0ea7b4b..c58a4150 100644
--- a/app/models/item.rb
+++ b/app/models/item.rb
@@ -1,3 +1,6 @@
+require "async"
+require "async/barrier"
+
 class Item < ApplicationRecord
   include PrettyParam
   
@@ -114,15 +117,25 @@ class Item < ApplicationRecord
 
   def nc_trade_value
     return nil unless nc?
-    begin
+
+    # Load the trade value, if we haven't already. Note that, because the trade
+    # value may be nil, we also save an explicit boolean for whether we've
+    # already looked it up, rather than checking if the saved value is empty.
+    return @nc_trade_value if @nc_trade_value_loaded
+
+    @nc_trade_value = begin
+      Rails.logger.debug "Item #{id} (#{name}) <lookup>"
       OwlsValueGuide.find_by_name(name)
     rescue OwlsValueGuide::NotFound => error
       Rails.logger.debug("No NC trade value listed for #{name} (#{id})")
-      return nil
+      nil
     rescue OwlsValueGuide::NetworkError => error
       Rails.logger.error("Couldn't load nc_trade_value: #{error.full_message}")
-      return nil
+      nil
     end
+
+    @nc_trade_value_loaded = true
+    @nc_trade_value
   end
   
   # Return an OrderedHash mapping users to the number of times they
@@ -599,6 +612,27 @@ class Item < ApplicationRecord
     end
   end
 
+  def self.preload_nc_trade_values(items)
+    # Only allow 10 trade values to be loaded at a time.
+    barrier = Async::Barrier.new
+    semaphore = Async::Semaphore.new(10, parent: barrier)
+
+    Sync do
+      # Load all the trade values in concurrent async tasks. (The
+      # `nc_trade_value` caches the value in the Item object.)
+      items.each do |item|
+        semaphore.async { item.nc_trade_value }
+      end
+
+      # Wait until all tasks are done.
+      barrier.wait
+    ensure
+      barrier.stop # If something goes wrong, clean up all tasks.
+    end
+
+    items
+  end
+
   def self.collection_from_pet_type_and_registries(pet_type, info_registry, asset_registry, scope=Item.all)
     # bear in mind that registries are arrays with many nil elements,
     # due to how the parser works