Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds content state comparison functionality to autosave interval #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rider-griffin
Copy link

@rider-griffin rider-griffin commented Nov 13, 2020

The Issue

Because nested sub-contents do not trigger their respective XAPI verbs, which in turn trigger save events, we have tuned down out autosave interval on our server to be extremely frequent to prevent the loss of user data and progress. This is undesirable, as:

  • Decreasing the autosave interval results in unnecessary server load.
  • Misleading UI feedback to the user in the form of frequent "Saving..." notifications, even when the user is idle.
  • For various content types that use sub-contents, such as Question Set, users that leave the content page before autosave triggers will lose progress.

Solution

By taking advantage of the existing auto-save functionality, user data can be saved conditionally by comparing the current state of the content with a previous state of the content. If the user has in any way interacted with the content, then the user data should be set and a previous state should be assigned for the next comparison. If the states are equal and unchanged, then the setting of user data does not need to be performed. The scope of the state comparison function is as follows:

  • The basis for the comparison algorithm is drawn from underscore-esm's deepEq algorithm, which recursively compares two objects and determines if they are equivalent.
  • The algorithm is designed around JSON-friendly objects (due to how the H5P.setUserData function handles states) but can compare complex objects such as circular ones.
  • The comparison returns once the first diff is found, which will reduce the time needed to compare large state objects.

Because the autosave will rely upon a state difference, the interval then becomes less honest. The cons of this implementation are that the algorithm will have to compare states based on the autosave interval, therefore increasing computational requirements given an extremely small interval and extremely large and nested state object. The pros, however, are:

  • Server hosts can comfortably decrease the autosave interval without worrying about large computational drawbacks.
  • Unnecessary saves will not be triggered, given any autosave interval, as long as the states have not changed.
  • The "Saving..." UI feedback will no longer appear unless the user has made changes/progress to the content.

@otacke
Copy link
Contributor

otacke commented Nov 18, 2020

@rider-griffin Have you considered to compute, store and compare checksums of the state objects instead of comparing them property by property?

@rider-griffin
Copy link
Author

@otacke
With checksums into consideration, you could use them to compare the state objects, yet I still believe that a deep comparison fits the case better here. Arguably, both comparison methods would perform similarly in regards to time complexity in which both run roughly in linear time. A checksum comparison would require additional overhead as to which type of checksum to be generated, i.e. a SHA hash of the state's JSON representation. The big concern here is that a checksum might yield two different results for two objects that are semantically the same given different key orderings, so either way the properties would have to be enumerated in insertion order. Both methods get the same job done in roughly the same amount of time, but I would argue that deep equal is a bit more robust given larger and complex state objects. I could refactor to perform a checksum-comparison if wanted, however from my perspective I see no outlying advantages to using them over a deep compare.

@otacke
Copy link
Contributor

otacke commented Nov 21, 2020

@rider-griffin I wasn't thinking of something fancy like SHA where a simple hash could work and I don't think that there'll be a problem with property order, but I get your point.

@rider-griffin rider-griffin force-pushed the feature/compare-states-before-saving branch from 28e32f6 to 9705f84 Compare October 27, 2021 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants