[apps/escher/ion] Remove Graph memoized models on storage change

apps/apps_container.cpp

@@ -74,6 +74,7 @@ AppsContainer::AppsContainer() :
 {
   m_emptyBatteryWindow.setFrame(KDRect(0, 0, Ion::Display::Width, Ion::Display::Height));
   Poincare::Expression::setCircuitBreaker(AppsContainer::poincareCircuitBreaker);
+  Ion::Storage::sharedStorage()->setDelegate(this);
 }
 
 bool AppsContainer::poincareCircuitBreaker() {
@@ -293,9 +294,9 @@ void AppsContainer::examDeactivatingPopUpIsDismissed() {
   }
 }
 
-void AppsContainer::storageDidChange(const Ion::Storage * storage) {
-  for (int i = 0; i < numberOfApps(); i++) {
-    appSnapshotAtIndex(i)->storageDidChange();
+void AppsContainer::storageDidChangeForRecord(const Ion::Storage::Record record) {
+  if (activeApp()) {
+    activeApp()->snapshot()->storageDidChangeForRecord(record);
   }
 }
 

apps/apps_container.h

@@ -56,7 +56,7 @@ public:
   // Exam pop-up controller delegate
   void examDeactivatingPopUpIsDismissed() override;
   // Ion::StorageDelegate
-  void storageDidChange(const Ion::Storage * storage) override;
+  void storageDidChangeForRecord(const Ion::Storage::Record record) override;
 protected:
   Home::App::Snapshot * homeAppSnapshot() { return &m_homeSnapshot; }
 private:

apps/graph/app.cpp

@@ -37,6 +37,10 @@ void App::Snapshot::reset() {
   m_graphRange.setDefault();
 }
 
+void App::Snapshot::storageDidChangeForRecord(const Ion::Storage::Record record) {
+  m_functionStore.storageDidChangeForRecord(record);
+}
+
 App::Descriptor * App::Snapshot::descriptor() {
   static Descriptor descriptor;
   return &descriptor;

apps/graph/app.h

@@ -23,6 +23,7 @@ public:
     Snapshot();
     App * unpack(Container * container) override;
     void reset() override;
+    void storageDidChangeForRecord(const Ion::Storage::Record record) override;
     Descriptor * descriptor() override;
     StorageCartesianFunctionStore * functionStore();
     Shared::InteractiveCurveViewRange * graphRange();

apps/shared/storage_expression_model_store.cpp

@@ -16,18 +16,9 @@ Ion::Storage::Record StorageExpressionModelStore::recordAtIndex(int i) const {
 }
 
 StorageExpressionModel * StorageExpressionModelStore::privateModelForRecord(Ion::Storage::Record record) const {
-  uint32_t currentStorageChecksum = Ion::Storage::sharedStorage()->checksum();
-  /* If the storage changed since last call to modelForRecord, we invalid all
-   * memoized models. Indeed, if f(x) = A+x, and A changed, f(x) memoization
-   *  which stores the reduced expression of f is outdated. */
-  if (currentStorageChecksum != m_storageChecksum) {
-    resetMemoizedModels();
-    m_storageChecksum = currentStorageChecksum;
-  } else {
-    for (int i = 0; i < k_maxNumberOfMemoizedModels; i++) {
-      if (!memoizedModelAtIndex(i)->isNull() && *memoizedModelAtIndex(i) == record) {
-        return memoizedModelAtIndex(i);
-      }
+  for (int i = 0; i < k_maxNumberOfMemoizedModels; i++) {
+    if (!memoizedModelAtIndex(i)->isNull() && *memoizedModelAtIndex(i) == record) {
+      return memoizedModelAtIndex(i);
     }
   }
   setMemoizedModelAtIndex(m_oldestMemoizedIndex, record);
@@ -47,9 +38,7 @@ void StorageExpressionModelStore::removeModel(Ion::Storage::Record record) {
 }
 
 void StorageExpressionModelStore::tidy() {
-  for (int i = 0; i < k_maxNumberOfMemoizedModels; i++) {
-    memoizedModelAtIndex(i)->tidy();
-  }
+  resetMemoizedModelsExceptRecord();
 }
 
 int StorageExpressionModelStore::numberOfModelsSatisfyingTest(ModelTest test) const {
@@ -86,10 +75,12 @@ Ion::Storage::Record StorageExpressionModelStore::recordStatifyingTestAtIndex(in
   return Ion::Storage::Record();
 }
 
-void StorageExpressionModelStore::resetMemoizedModels() const {
+void StorageExpressionModelStore::resetMemoizedModelsExceptRecord(const Ion::Storage::Record record) const {
   Ion::Storage::Record emptyRecord;
   for (int i = 0; i < k_maxNumberOfMemoizedModels; i++) {
-    setMemoizedModelAtIndex(i, emptyRecord);
+    if (*memoizedModelAtIndex(i) != record) {
+      setMemoizedModelAtIndex(i, emptyRecord);
+    }
   }
 }
 

apps/shared/storage_expression_model_store.h

@@ -29,6 +29,7 @@ public:
 
   // Other
   void tidy();
+  void storageDidChangeForRecord(const Ion::Storage::Record record) const { resetMemoizedModelsExceptRecord(record); }
 protected:
   constexpr static int k_maxNumberOfMemoizedModels = 5;
   typedef bool (*ModelTest)(StorageExpressionModel * model);
@@ -36,7 +37,7 @@ protected:
   Ion::Storage::Record recordStatifyingTestAtIndex(int i, ModelTest test) const;
   StorageExpressionModel * privateModelForRecord(Ion::Storage::Record record) const;
 private:
-  void resetMemoizedModels() const;
+  void resetMemoizedModelsExceptRecord(const Ion::Storage::Record record = Ion::Storage::Record()) const;
   virtual void setMemoizedModelAtIndex(int cacheIndex, Ion::Storage::Record) const = 0;
   virtual StorageExpressionModel * memoizedModelAtIndex(int cacheIndex) const = 0;
   virtual const char * modelExtension() const = 0;
@@ -46,8 +47,6 @@ private:
    * same time. Otherwise, we should use a queue to decide which was the last
    * memoized model. */
   mutable int m_oldestMemoizedIndex;
-  mutable uint32_t m_storageChecksum;
-
 };
 
 }

escher/include/escher/app.h

@@ -34,7 +34,7 @@ public:
     void pack(App * app);
     /* reset all instances to their initial values */
     virtual void reset();
-    virtual void storageDidChange() {}
+    virtual void storageDidChangeForRecord(Ion::Storage::Record) {}
     virtual Descriptor * descriptor() = 0;
 #if EPSILON_GETOPT
     virtual void setOpt(const char * name, char * value) {}

ion/include/ion/storage.h

@@ -81,7 +81,7 @@ public:
 
   // Delegate
   void setDelegate(StorageDelegate * delegate) { m_delegate = delegate; }
-  void notifyChangeToDelegate() const;
+  void notifyChangeToDelegate(const Record r = Record()) const;
 
   int numberOfRecordsWithExtension(const char * extension);
   static bool FullNameHasExtension(const char * fullName, const char * extension, size_t extensionLength);
@@ -155,14 +155,16 @@ private:
 };
 
 /* Some apps memoize records and need to be notified when a record might have
- * become invalid. We could have computed and compared the checksum of the
- * storage to detect storage invalidity, but profiling showed that this slows
- * down the execution (for example when scrolling the functions list).
+ * become invalid. For instance in the Graph app, if f(x) = A+x, and A changed,
+ * f(x) memoization which stores the reduced expression of f is outdated.
+ * We could have computed and compared the checksum of the storage to detect
+ * storage invalidity, but profiling showed that this slows down the execution
+ * (for example when scrolling the functions list).
  * We thus decided to notify a delegate when the storage changes. */
 
 class StorageDelegate {
 public:
-  virtual void storageDidChange(const Storage * storage) = 0;
+  virtual void storageDidChangeForRecord(const Storage::Record record) = 0;
 };
 
 }

ion/src/shared/storage.cpp

@@ -90,9 +90,9 @@ uint32_t Storage::checksum() {
   return Ion::crc32PaddedString(m_buffer, endBuffer()-m_buffer);
 }
 
-void Storage::notifyChangeToDelegate() const {
+void Storage::notifyChangeToDelegate(const Record record) const {
   if (m_delegate != nullptr) {
-    m_delegate->storageDidChange(this);
+    m_delegate->storageDidChangeForRecord(record);
   }
 }
 
@@ -105,7 +105,8 @@ Storage::Record::ErrorStatus Storage::createRecordWithFullName(const char * full
     return Record::ErrorStatus::NameTaken;
   }
   // Find the end of data
-  char * newRecord = endBuffer();
+  char * newRecordAddress = endBuffer();
+  char * newRecord = newRecordAddress;
   // Fill totalSize
   newRecord += overrideSizeAtPosition(newRecord, (record_size_t)recordSize);
   // Fill name
@@ -114,7 +115,7 @@ Storage::Record::ErrorStatus Storage::createRecordWithFullName(const char * full
   newRecord += overrideValueAtPosition(newRecord, data, size);
   // Next Record is null-sized
   overrideSizeAtPosition(newRecord, 0);
-  notifyChangeToDelegate();
+  notifyChangeToDelegate(Record(fullName));
   return Record::ErrorStatus::None;
 }
 
@@ -127,7 +128,8 @@ Storage::Record::ErrorStatus Storage::createRecordWithExtension(const char * bas
     return Record::ErrorStatus::NameTaken;
   }
   // Find the end of data
-  char * newRecord = endBuffer();
+  char * newRecordAddress = endBuffer();
+  char * newRecord = newRecordAddress;
   // Fill totalSize
   newRecord += overrideSizeAtPosition(newRecord, (record_size_t)recordSize);
   // Fill name
@@ -136,7 +138,7 @@ Storage::Record::ErrorStatus Storage::createRecordWithExtension(const char * bas
   newRecord += overrideValueAtPosition(newRecord, data, size);
   // Next Record is null-sized
   overrideSizeAtPosition(newRecord, 0);
-  notifyChangeToDelegate();
+  notifyChangeToDelegate(Record(fullNameOfRecordStarting(newRecordAddress)));
   return Record::ErrorStatus::None;
 }
 
@@ -255,7 +257,7 @@ Storage::Record::ErrorStatus Storage::setFullNameOfRecord(const Record record, c
       }
       overrideSizeAtPosition(p, newRecordSize);
       overrideFullNameAtPosition(p+sizeof(record_size_t), fullName);
-      notifyChangeToDelegate();
+      notifyChangeToDelegate(record);
       return Record::ErrorStatus::None;
     }
   }
@@ -278,7 +280,7 @@ Storage::Record::ErrorStatus Storage::setBaseNameWithExtensionOfRecord(Record re
       }
       overrideSizeAtPosition(p, newRecordSize);
       overrideBaseNameWithExtensionAtPosition(p+sizeof(record_size_t), baseName, extension);
-      notifyChangeToDelegate();
+      notifyChangeToDelegate(record);
       return Record::ErrorStatus::None;
     }
   }
@@ -311,7 +313,7 @@ Storage::Record::ErrorStatus Storage::setValueOfRecord(Record record, Record::Da
       record_size_t fullNameSize = strlen(fullName)+1;
       overrideSizeAtPosition(p, newRecordSize);
       overrideValueAtPosition(p+sizeof(record_size_t)+fullNameSize, data.buffer, data.size);
-      notifyChangeToDelegate();
+      notifyChangeToDelegate(record);
       return Record::ErrorStatus::None;
     }
   }