Allow savestate undo disable for pcsx_rearmed on old3ds (#18755)

* Makefile.ctr: enable gc-sections

This frees ~160k of RAM without any downsides that I know of.
It's not much, but helps pcsx_rearmed on old3ds where it barely fits in RAM.

* remove some dupe logic

Regardless if `save_state_in_background` is enabled or not, it does
exactly the same so the check can be removed.

* optimize RAM state load

content_load_state_from_ram() doesn't need the backup copy,
content_save_state() never touches the ram_buf. It seems the whole
logic was copy-pasted from content_undo_load_state() without thinking.

* avoid useless memory use and copying

Just take the source pointer instead of making a copy and then freeing it.
This is a big deal on slow platforms having to deal with large states,
like psx on old3ds.

* allow the core to disable the savestate undo feature

The undo feature may amount to at least 4 simultaneous buffers:
save undo, load undo, the current save/load buffer, temporary buffer.

When emulating a system with relatively large states on a system with
little RAM (like PSX on old3ds) that amounts to something like ~18MB of
~42MB available to the core. Under memory pressure the libctru variation
in RetroArch tries to salvage some memory from the main stack and linear
memory, but since those are variable, they start to overlap creating
instability.

The solution is to just disable the undo feature.

Author: notaz

Makefile.ctr

@@ -200,6 +200,9 @@ ASFLAGS	 := -g $(ARCH) -O3
 LDFLAGS  += -specs=ctr/3dsx_custom.specs -g $(ARCH) -Wl,-Map,$(notdir $*.map)
 CFLAGS   += -std=gnu99
 
+CFLAGS   += -ffunction-sections -fdata-sections
+LDFLAGS  += -Wl,--gc-sections
+
 LIB_CORE      :=
 LIB_CORE_FULL :=
 

command.c

@@ -1351,8 +1351,12 @@ static size_t command_event_save_config(const char *config_path,
 static size_t command_event_undo_save_state(char *s, size_t len)
 {
    if (content_undo_save_buf_is_empty())
-      return strlcpy(s,
-         msg_hash_to_str(MSG_NO_SAVE_STATE_HAS_BEEN_OVERWRITTEN_YET), len);
+   {
+      enum msg_hash_enums msg = content_undo_save_disabled()
+         ? MSG_CORE_DOES_NOT_SUPPORT_SAVESTATE_UNDO
+         : MSG_NO_SAVE_STATE_HAS_BEEN_OVERWRITTEN_YET;
+      return strlcpy(s, msg_hash_to_str(msg), len);
+   }
    if (!content_undo_save_state())
       return strlcpy(s,
          msg_hash_to_str(MSG_FAILED_TO_UNDO_SAVE_STATE), len);
@@ -1363,9 +1367,12 @@ static size_t command_event_undo_save_state(char *s, size_t len)
 static size_t command_event_undo_load_state(char *s, size_t len)
 {
    if (content_undo_load_buf_is_empty())
-      return strlcpy(s,
-         msg_hash_to_str(MSG_NO_STATE_HAS_BEEN_LOADED_YET),
-         len);
+   {
+      enum msg_hash_enums msg = content_undo_save_disabled()
+         ? MSG_CORE_DOES_NOT_SUPPORT_SAVESTATE_UNDO
+         : MSG_NO_STATE_HAS_BEEN_LOADED_YET;
+      return strlcpy(s, msg_hash_to_str(msg), len);
+   }
    if (!content_undo_load_state())
       return snprintf(s, len, "%s \"%s\".",
             msg_hash_to_str(MSG_FAILED_TO_UNDO_LOAD_STATE),

content.h

@@ -103,9 +103,10 @@ bool content_init(void);
 /* Resets the state and savefile backup buffers */
 void content_reset_savestate_backups(void);
 
-/* Checks if the buffers are empty */
+/* Checks if the buffers are empty, or undo feature is disabled */
 bool content_undo_load_buf_is_empty(void);
 bool content_undo_save_buf_is_empty(void);
+bool content_undo_save_disabled(void);
 
 /* Clears the pending subsystem rom buffer */
 bool content_is_subsystem_pending_load(void);

intl/msg_hash_us.h

@@ -14195,6 +14195,10 @@ MSG_HASH(
    MSG_CORE_DOES_NOT_SUPPORT_SAVESTATES,
    "Core does not support save states."
    )
+MSG_HASH(
+   MSG_CORE_DOES_NOT_SUPPORT_SAVESTATE_UNDO,
+   "Core does not support save state undo."
+   )
 MSG_HASH(
    MSG_CORE_DOES_NOT_SUPPORT_DISK_OPTIONS,
    "Core does not support Disc Control."

msg_hash.h

@@ -513,6 +513,7 @@ enum msg_hash_enums
    MSG_SAVED_STATE_TO_SLOT,
    MSG_SAVED_STATE_TO_SLOT_AUTO,
    MSG_CORE_DOES_NOT_SUPPORT_SAVESTATES,
+   MSG_CORE_DOES_NOT_SUPPORT_SAVESTATE_UNDO,
    MSG_FAILED_TO_LOAD_STATE,
    MSG_FAILED_TO_UNDO_LOAD_STATE,
    MSG_FAILED_TO_UNDO_SAVE_STATE,

retroarch.h

@@ -71,6 +71,11 @@
                                             * 3 - Late
                                             */
 
+#define RETRO_ENVIRONMENT_SET_SAVE_STATE_DISABLE_UNDO (5 | RETRO_ENVIRONMENT_RETROARCH_START_BLOCK)
+                                            /* bool * --
+                                            * If true, disables the save state save/load undo feature to conserve memory.
+                                            */
+
 #define DRIVERS_CMD_ALL \
       ( DRIVER_AUDIO_MASK \
       | DRIVER_MICROPHONE_MASK \

runloop.c

@@ -3383,6 +3383,16 @@ bool runloop_environment_cb(unsigned cmd, void *data)
          }
          break;
 
+      case RETRO_ENVIRONMENT_SET_SAVE_STATE_DISABLE_UNDO:
+         {
+            bool state = *(const bool*)data;
+
+            RARCH_LOG("[Environ] RETRO_ENVIRONMENT_SET_SAVE_STATE_DISABLE_UNDO: %s.\n", state ? "yes" : "no");
+
+            set_save_state_disable_undo(state);
+         }
+         break;
+
       case RETRO_ENVIRONMENT_SET_CONTENT_INFO_OVERRIDE:
          {
             const struct retro_system_content_info_override *overrides =

tasks/task_save.c

@@ -134,6 +134,7 @@ static struct save_state_buf undo_load_buf;
 static struct ram_save_state_buf ram_buf;
 
 static bool save_state_in_background       = false;
+static bool save_state_disable_undo        = false;
 
 typedef struct rastate_size_info
 {
@@ -1012,19 +1013,12 @@ static void content_load_state_cb(retro_task_t *task,
    {
       /* If we were previously backing up a file, let go of it first */
       if (undo_save_buf.data)
-      {
          free(undo_save_buf.data);
-         undo_save_buf.data = NULL;
-      }
-
-      if (!(undo_save_buf.data = malloc(_len)))
-         goto error;
 
-      memcpy(undo_save_buf.data, buf, _len);
+      undo_save_buf.data = buf;
       undo_save_buf.size = _len;
       strlcpy(undo_save_buf.path, load_data->path, sizeof(undo_save_buf.path));
 
-      free(buf);
       free(load_data);
       return;
    }
@@ -1403,6 +1397,9 @@ bool content_save_state(const char *path, bool save_to_disk)
    size_t _len;
    void *data  = NULL;
 
+   if (!save_to_disk && save_state_disable_undo)
+      return false;
+
    if (!core_info_current_supports_savestate())
    {
       RARCH_LOG("[State] %s\n",
@@ -1433,7 +1430,7 @@ bool content_save_state(const char *path, bool save_to_disk)
 
    if (save_to_disk)
    {
-      if (path_is_valid(path))
+      if (!save_state_disable_undo && path_is_valid(path))
       {
          /* Before overwriting the savestate file, load it into a buffer
          to allow undo_save_state() to work */
@@ -1463,19 +1460,9 @@ bool content_save_state(const char *path, bool save_to_disk)
 
       /* If we were holding onto an old state already, clean it up first */
       if (undo_load_buf.data)
-      {
          free(undo_load_buf.data);
-         undo_load_buf.data = NULL;
-      }
 
-      if (!(undo_load_buf.data = malloc(_len)))
-      {
-         free(data);
-         return false;
-      }
-
-      memcpy(undo_load_buf.data, data, _len);
-      free(data);
+      undo_load_buf.data = data;
       undo_load_buf.size = _len;
       strlcpy(undo_load_buf.path, path, sizeof(undo_load_buf.path));
    }
@@ -1664,6 +1651,11 @@ bool content_undo_save_buf_is_empty(void)
    return undo_save_buf.data == NULL || undo_save_buf.size == 0;
 }
 
+bool content_undo_save_disabled(void)
+{
+   return save_state_disable_undo;
+}
+
 /**
  * content_load_state_from_ram:
  * Load a state from RAM.
@@ -1672,9 +1664,7 @@ bool content_undo_save_buf_is_empty(void)
  **/
 bool content_load_state_from_ram(void)
 {
-   size_t temp_data_size;
    bool ret        = false;
-   void* temp_data = NULL;
 
    if (!core_info_current_supports_savestate())
    {
@@ -1691,21 +1681,10 @@ bool content_load_state_from_ram(void)
          (unsigned)ram_buf.state_buf.size,
          msg_hash_to_str(MSG_BYTES));
 
-   /* We need to make a temporary copy of the buffer, to allow the swap below */
-   temp_data       = malloc(ram_buf.state_buf.size);
-   temp_data_size  = ram_buf.state_buf.size;
-   memcpy(temp_data, ram_buf.state_buf.data, ram_buf.state_buf.size);
-
-   /* Swap the current state with the backup state. This way, we can undo
-   what we're undoing */
+   /* Backup the current state so we can undo this load */
    content_save_state("RAM", false);
 
-   ret             = content_deserialize_state(temp_data, temp_data_size);
-
-   /* Clean up the temporary copy */
-   free(temp_data);
-   temp_data       = NULL;
-
+   ret = content_deserialize_state(ram_buf.state_buf.data, ram_buf.state_buf.size);
    if (!ret)
    {
       RARCH_ERR("[State] %s.\n",
@@ -1735,50 +1714,32 @@ bool content_save_state_to_ram(void)
    }
 
    _len = core_serialize_size();
-
    if (_len == 0)
       return false;
 
-   if (!save_state_in_background)
-   {
-      if (!(data = content_get_serialized_data(&_len)))
-      {
-         RARCH_ERR("[State] %s.\n",
-               msg_hash_to_str(MSG_FAILED_TO_SAVE_SRAM));
-         return false;
-      }
-
-      RARCH_LOG("[State] %s, %u %s.\n",
-            msg_hash_to_str(MSG_SAVING_STATE),
-            (unsigned)_len,
-            msg_hash_to_str(MSG_BYTES));
-   }
-
-   if (!data)
-   {
-      if (!(data = content_get_serialized_data(&_len)))
-      {
-         RARCH_ERR("[State] %s.\n",
-               msg_hash_to_str(MSG_FAILED_TO_SAVE_SRAM));
-         return false;
-      }
-   }
+   RARCH_LOG("[State] %s, %u %s.\n",
+         msg_hash_to_str(MSG_SAVING_STATE),
+         (unsigned)_len,
+         msg_hash_to_str(MSG_BYTES));
 
-   /* If we were holding onto an old state already, clean it up first */
-   if (ram_buf.state_buf.data)
+   if (save_state_disable_undo && ram_buf.state_buf.data)
    {
+      /* Undo off means lack of memory, free before we alloc the new one */
       free(ram_buf.state_buf.data);
       ram_buf.state_buf.data = NULL;
    }
 
-   if (!(ram_buf.state_buf.data = malloc(_len)))
+   if (!(data = content_get_serialized_data(&_len)))
    {
-      free(data);
+      RARCH_ERR("[State] %s.\n",
+            msg_hash_to_str(MSG_FAILED_TO_SAVE_SRAM));
       return false;
    }
 
-   memcpy(ram_buf.state_buf.data, data, _len);
-   free(data);
+   if (ram_buf.state_buf.data)
+      free(ram_buf.state_buf.data);
+
+   ram_buf.state_buf.data = data;
    ram_buf.state_buf.size = _len;
    ram_buf.to_write_file  = true;
 
@@ -1827,3 +1788,8 @@ void set_save_state_in_background(bool state)
 {
    save_state_in_background = state;
 }
+
+void set_save_state_disable_undo(bool disable)
+{
+   save_state_disable_undo = disable;
+}

tasks/tasks_internal.h

@@ -261,6 +261,7 @@ bool input_autoconfigure_disconnect(
       unsigned port, const char *name);
 
 void set_save_state_in_background(bool state);
+void set_save_state_disable_undo(bool disable);
 
 #ifdef HAVE_CDROM
 void task_push_cdrom_dump(const char *drive);