Skip to content

Commit ce60bee

Browse files
tannewttannewt
authored
Merge pull request adafruit#6651 from tannewt/refine_select_task
Improve web workflow responsiveness
2 parents 6463755 + bb734c7 commit ce60bee

File tree

3 files changed

+80
-26
lines changed

3 files changed

+80
-26
lines changed

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

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -60,32 +60,35 @@ STATIC void socket_select_task(void *arg) {
6060
FD_SET(socket_change_fd, &errfds);
6161
int max_fd = socket_change_fd;
6262
for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
63-
if (open_socket_fds[i] < 0) {
63+
int sockfd = open_socket_fds[i];
64+
if (sockfd < 0) {
6465
continue;
6566
}
66-
max_fd = MAX(max_fd, open_socket_fds[i]);
67-
FD_SET(open_socket_fds[i], &readfds);
68-
FD_SET(open_socket_fds[i], &errfds);
67+
max_fd = MAX(max_fd, sockfd);
68+
FD_SET(sockfd, &readfds);
69+
FD_SET(sockfd, &errfds);
6970
}
7071

7172
int num_triggered = select(max_fd + 1, &readfds, NULL, &errfds, NULL);
72-
if (num_triggered < 0) {
73-
// Maybe bad file descriptor
74-
for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
75-
int sockfd = open_socket_fds[i];
76-
if (sockfd < 0) {
77-
continue;
78-
}
79-
if (FD_ISSET(sockfd, &errfds)) {
80-
int err;
81-
int optlen = sizeof(int);
82-
int ret = getsockopt(sockfd, SOL_SOCKET, SO_ERROR, &err, (socklen_t *)&optlen);
83-
if (ret < 0) {
84-
open_socket_fds[i] = -1;
85-
// Try again.
86-
continue;
87-
}
88-
}
73+
// Check for bad file descriptor and queue up the background task before
74+
// circling around.
75+
if (num_triggered == -1 && errno == EBADF) {
76+
// One for the change fd and one for the closed socket.
77+
num_triggered = 2;
78+
}
79+
// Try and find the bad file and remove it from monitoring.
80+
for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
81+
int sockfd = open_socket_fds[i];
82+
if (sockfd < 0) {
83+
continue;
84+
}
85+
int err;
86+
int optlen = sizeof(int);
87+
int ret = getsockopt(sockfd, SOL_SOCKET, SO_ERROR, &err, (socklen_t *)&optlen);
88+
if (ret < 0) {
89+
open_socket_fds[i] = -1;
90+
// Raise num_triggered so that we skip the assert and queue the background task.
91+
num_triggered = 2;
8992
}
9093
}
9194
assert(num_triggered >= 0);
@@ -117,13 +120,13 @@ void socket_user_reset(void) {
117120
user_socket[i] = false;
118121
}
119122
socket_change_fd = eventfd(0, 0);
120-
// This task runs at a lower priority than CircuitPython and is used to wake CircuitPython
121-
// up when any open sockets have data to read. It allows us to sleep otherwise.
123+
// Run this at the same priority as CP so that the web workflow background task can be
124+
// queued while CP is running. Both tasks can still sleep and, therefore, sleep overall.
122125
(void)xTaskCreateStaticPinnedToCore(socket_select_task,
123126
"socket_select",
124127
2 * configMINIMAL_STACK_SIZE,
125128
NULL,
126-
0, // Run this at IDLE priority. We only need it when CP isn't running (at 1).
129+
uxTaskPriorityGet(NULL),
127130
socket_select_stack,
128131
&socket_select_task_handle,
129132
xPortGetCoreID());

‎supervisor/shared/web_workflow/web_workflow.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,6 +1265,9 @@ void supervisor_web_workflow_background(void) {
12651265
// If we have a request in progress, continue working on it.
12661266
if (common_hal_socketpool_socket_get_connected(&active)) {
12671267
_process_request(&active, &active_request);
1268+
} else {
1269+
// Close the active socket if it is no longer connected.
1270+
common_hal_socketpool_socket_close(&active);
12681271
}
12691272
}
12701273

‎tools/ci_set_matrix.py‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import yaml
2222

2323
import build_board_info
24+
from shared_bindings_matrix import get_settings_from_makefile
2425

2526
PORT_TO_ARCH = {
2627
"atmel-samd": "arm",
@@ -53,6 +54,7 @@ def set_boards_to_build(build_all):
5354
all_board_ids = set()
5455
port_to_boards = {}
5556
board_to_port = {}
57+
board_settings = {}
5658
for board_id in boards_info_json:
5759
info = boards_info_json[board_id]
5860
if info.get("alias", False):
@@ -70,6 +72,9 @@ def set_boards_to_build(build_all):
7072
boards_to_build = set()
7173
board_pattern = re.compile(r"^ports/[^/]+/boards/([^/]+)/")
7274
port_pattern = re.compile(r"^ports/([^/]+)/")
75+
module_pattern = re.compile(
76+
r"^(ports/[^/]+/common-hal|shared-bindings|shared-module)/([^/]+)/"
77+
)
7378
for p in changed_files:
7479
# See if it is board specific
7580
board_matches = board_pattern.search(p)
@@ -80,7 +85,8 @@ def set_boards_to_build(build_all):
8085

8186
# See if it is port specific
8287
port_matches = port_pattern.search(p)
83-
if port_matches:
88+
module_matches = module_pattern.search(p)
89+
if port_matches and not module_matches:
8490
port = port_matches.group(1)
8591
if port != "unix":
8692
boards_to_build.update(port_to_boards[port])
@@ -94,14 +100,56 @@ def set_boards_to_build(build_all):
94100
if p.startswith("tests"):
95101
continue
96102

103+
# As a (nearly) last resort, for some certain files, we compute the settings from the
104+
# makefile for each board and determine whether to build them that way.
105+
if p.startswith("frozen") or p.startswith("supervisor") or module_matches:
106+
for board in all_board_ids:
107+
if board not in board_settings:
108+
board_settings[board] = get_settings_from_makefile(
109+
"../ports/" + board_to_port[board], board
110+
)
111+
settings = board_settings[board]
112+
113+
# Check frozen files to see if they are in each board.
114+
frozen = settings.get("FROZEN_MPY_DIRS", "")
115+
if frozen and p.startswith("frozen") and p in frozen:
116+
boards_to_build.add(board)
117+
continue
118+
119+
# Check supervisor files. This is useful for limiting workflow changes to the
120+
# relevant boards.
121+
supervisor = settings["SRC_SUPERVISOR"]
122+
if p.startswith("supervisor"):
123+
if p in supervisor:
124+
boards_to_build.add(board)
125+
continue
126+
127+
web_workflow = settings["CIRCUITPY_WEB_WORKFLOW"]
128+
while web_workflow.startswith("$("):
129+
web_workflow = settings[web_workflow[2:-1]]
130+
if (
131+
p.startswith("supervisor/shared/web_workflow/static/")
132+
and web_workflow != "0"
133+
):
134+
boards_to_build.add(board)
135+
continue
136+
137+
# Check module matches
138+
if module_matches:
139+
module = module_matches.group(2) + "/"
140+
if module in settings["SRC_PATTERNS"]:
141+
boards_to_build.add(board)
142+
continue
143+
continue
144+
97145
# Otherwise build it all
98146
boards_to_build = all_board_ids
99147
break
100148

101149
# Split boards by architecture.
102150
print("Building boards:")
103151
arch_to_boards = {"aarch": [], "arm": [], "riscv": [], "espressif": []}
104-
for board in boards_to_build:
152+
for board in sorted(boards_to_build):
105153
print(" ", board)
106154
port = board_to_port.get(board)
107155
# A board can appear due to its _deletion_ (rare)

0 commit comments

Comments
 (0)