Merge pull request #6651 from tannewt/refine_select_task

Improve web workflow responsiveness

Author: tannewt

ports/espressif/common-hal/socketpool/Socket.c

@@ -60,32 +60,35 @@ STATIC void socket_select_task(void *arg) {
         FD_SET(socket_change_fd, &errfds);
         int max_fd = socket_change_fd;
         for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
-            if (open_socket_fds[i] < 0) {
+            int sockfd = open_socket_fds[i];
+            if (sockfd < 0) {
                 continue;
             }
-            max_fd = MAX(max_fd, open_socket_fds[i]);
-            FD_SET(open_socket_fds[i], &readfds);
-            FD_SET(open_socket_fds[i], &errfds);
+            max_fd = MAX(max_fd, sockfd);
+            FD_SET(sockfd, &readfds);
+            FD_SET(sockfd, &errfds);
         }
 
         int num_triggered = select(max_fd + 1, &readfds, NULL, &errfds, NULL);
-        if (num_triggered < 0) {
-            // Maybe bad file descriptor
-            for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
-                int sockfd = open_socket_fds[i];
-                if (sockfd < 0) {
-                    continue;
-                }
-                if (FD_ISSET(sockfd, &errfds)) {
-                    int err;
-                    int optlen = sizeof(int);
-                    int ret = getsockopt(sockfd, SOL_SOCKET, SO_ERROR, &err, (socklen_t *)&optlen);
-                    if (ret < 0) {
-                        open_socket_fds[i] = -1;
-                        // Try again.
-                        continue;
-                    }
-                }
+        // Check for bad file descriptor and queue up the background task before
+        // circling around.
+        if (num_triggered == -1 && errno == EBADF) {
+            // One for the change fd and one for the closed socket.
+            num_triggered = 2;
+        }
+        // Try and find the bad file and remove it from monitoring.
+        for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
+            int sockfd = open_socket_fds[i];
+            if (sockfd < 0) {
+                continue;
+            }
+            int err;
+            int optlen = sizeof(int);
+            int ret = getsockopt(sockfd, SOL_SOCKET, SO_ERROR, &err, (socklen_t *)&optlen);
+            if (ret < 0) {
+                open_socket_fds[i] = -1;
+                // Raise num_triggered so that we skip the assert and queue the background task.
+                num_triggered = 2;
             }
         }
         assert(num_triggered >= 0);
@@ -117,13 +120,13 @@ void socket_user_reset(void) {
             user_socket[i] = false;
         }
         socket_change_fd = eventfd(0, 0);
-        // This task runs at a lower priority than CircuitPython and is used to wake CircuitPython
-        // up when any open sockets have data to read. It allows us to sleep otherwise.
+        // Run this at the same priority as CP so that the web workflow background task can be
+        // queued while CP is running. Both tasks can still sleep and, therefore, sleep overall.
         (void)xTaskCreateStaticPinnedToCore(socket_select_task,
             "socket_select",
             2 * configMINIMAL_STACK_SIZE,
             NULL,
-            0, // Run this at IDLE priority. We only need it when CP isn't running (at 1).
+            uxTaskPriorityGet(NULL),
             socket_select_stack,
             &socket_select_task_handle,
             xPortGetCoreID());

supervisor/shared/web_workflow/web_workflow.c

@@ -1265,6 +1265,9 @@ void supervisor_web_workflow_background(void) {
     // If we have a request in progress, continue working on it.
     if (common_hal_socketpool_socket_get_connected(&active)) {
         _process_request(&active, &active_request);
+    } else {
+        // Close the active socket if it is no longer connected.
+        common_hal_socketpool_socket_close(&active);
     }
 }
 

tools/ci_set_matrix.py

@@ -21,6 +21,7 @@
 import yaml
 
 import build_board_info
+from shared_bindings_matrix import get_settings_from_makefile
 
 PORT_TO_ARCH = {
     "atmel-samd": "arm",
@@ -53,6 +54,7 @@ def set_boards_to_build(build_all):
     all_board_ids = set()
     port_to_boards = {}
     board_to_port = {}
+    board_settings = {}
     for board_id in boards_info_json:
         info = boards_info_json[board_id]
         if info.get("alias", False):
@@ -70,6 +72,9 @@ def set_boards_to_build(build_all):
         boards_to_build = set()
         board_pattern = re.compile(r"^ports/[^/]+/boards/([^/]+)/")
         port_pattern = re.compile(r"^ports/([^/]+)/")
+        module_pattern = re.compile(
+            r"^(ports/[^/]+/common-hal|shared-bindings|shared-module)/([^/]+)/"
+        )
         for p in changed_files:
             # See if it is board specific
             board_matches = board_pattern.search(p)
@@ -80,7 +85,8 @@ def set_boards_to_build(build_all):
 
             # See if it is port specific
             port_matches = port_pattern.search(p)
-            if port_matches:
+            module_matches = module_pattern.search(p)
+            if port_matches and not module_matches:
                 port = port_matches.group(1)
                 if port != "unix":
                     boards_to_build.update(port_to_boards[port])
@@ -94,14 +100,56 @@ def set_boards_to_build(build_all):
             if p.startswith("tests"):
                 continue
 
+            # As a (nearly) last resort, for some certain files, we compute the settings from the
+            # makefile for each board and determine whether to build them that way.
+            if p.startswith("frozen") or p.startswith("supervisor") or module_matches:
+                for board in all_board_ids:
+                    if board not in board_settings:
+                        board_settings[board] = get_settings_from_makefile(
+                            "../ports/" + board_to_port[board], board
+                        )
+                    settings = board_settings[board]
+
+                    # Check frozen files to see if they are in each board.
+                    frozen = settings.get("FROZEN_MPY_DIRS", "")
+                    if frozen and p.startswith("frozen") and p in frozen:
+                        boards_to_build.add(board)
+                        continue
+
+                    # Check supervisor files. This is useful for limiting workflow changes to the
+                    # relevant boards.
+                    supervisor = settings["SRC_SUPERVISOR"]
+                    if p.startswith("supervisor"):
+                        if p in supervisor:
+                            boards_to_build.add(board)
+                            continue
+
+                        web_workflow = settings["CIRCUITPY_WEB_WORKFLOW"]
+                        while web_workflow.startswith("$("):
+                            web_workflow = settings[web_workflow[2:-1]]
+                        if (
+                            p.startswith("supervisor/shared/web_workflow/static/")
+                            and web_workflow != "0"
+                        ):
+                            boards_to_build.add(board)
+                            continue
+
+                    # Check module matches
+                    if module_matches:
+                        module = module_matches.group(2) + "/"
+                        if module in settings["SRC_PATTERNS"]:
+                            boards_to_build.add(board)
+                            continue
+                continue
+
             # Otherwise build it all
             boards_to_build = all_board_ids
             break
 
     # Split boards by architecture.
     print("Building boards:")
     arch_to_boards = {"aarch": [], "arm": [], "riscv": [], "espressif": []}
-    for board in boards_to_build:
+    for board in sorted(boards_to_build):
         print(" ", board)
         port = board_to_port.get(board)
         # A board can appear due to its _deletion_ (rare)