Skip to content

servo harness: Reduce recursive execution for shutdown#42770

Merged
yezhizhen merged 1 commit intoservo:mainfrom
yezhizhen:servo-faster
Feb 23, 2026
Merged

servo harness: Reduce recursive execution for shutdown#42770
yezhizhen merged 1 commit intoservo:mainfrom
yezhizhen:servo-faster

Conversation

@yezhizhen
Copy link
Copy Markdown
Member

@yezhizhen yezhizhen commented Feb 23, 2026 •
edited
Loading

I like servo runner, or better known as its previous name servodriver to some people.
However, it is getting quite often that I use legacy, or better known as its previous name servo.
This is because servo (previously servodriver) harness shutdown very slowly due to its shutdown command #40455

  • This PR improves the shutdown command in the way as titled.
  • Avoids accidental Prockill that may happen after regular shutdown. This may be related to @jschwe's report that coverage for WPT still reports empty more often than it should.

Previously, shutdown is like:

 0:15.06 INFO Trying to shut down gracefully by extension command
 0:18.13 DEBUG Servo has shut down normally. HTTPConnectionPool(host='127.0.0.1', port=8378): Max retries exceeded with url: /status (Caused by NewConnectionError("HTTPConnection(host='127.0.0.1', port=8378): Failed to establish a new connection: [WinError 10061] No connection could be made because the target machine actively refused it"))
 0:18.13 DEBUG Stopping WebDriver
 0:20.19 DEBUG Servo has shut down normally. HTTPConnectionPool(host='127.0.0.1', port=8378): Max retries exceeded with url: /status (Caused by NewConnectionError("HTTPConnection(host='127.0.0.1', port=8378): Failed to establish a new connection: [WinError 10061] No connection could be made because the target machine actively refused it"))
 0:22.24 DEBUG Servo has shut down normally. HTTPConnectionPool(host='127.0.0.1', port=8378): Max retries exceeded with url: /status (Caused by NewConnectionError("HTTPConnection(host='127.0.0.1', port=8378): Failed to establish a new connection: [WinError 10061] No connection could be made because the target machine actively refused it"))
 0:14.99 DEBUG Hanging up on WebDriver session

Now:

 INFO Trying to shut down gracefully by extension command
 0:18.41 DEBUG Servo has shut down normally. HTTPConnectionPool(host='127.0.0.1', port=9098): Max retries exceeded with url: /status (Caused by NewConnectionError("HTTPConnection(host='127.0.0.1', port=9098): Failed to establish a new connection: [WinError 10061] No connection could be made because the target machine actively refused it"))
 0:18.41 DEBUG TestRunnerManager cleanup
 0:15.30 DEBUG Hanging up on WebDriver session

Testing: Locally, this reduce shutdown time from about 5 sec to 1 sec, counting from "Trying to shut down gracefully by extension command".
Try.

@yezhizhen yezhizhen requested a review from xiaochengh February 23, 2026 07:03
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 23, 2026
@servo-wpt-sync servo-wpt-sync mentioned this pull request Feb 23, 2026
Merged
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

servo-wpt-sync commented Feb 23, 2026

🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#57973) with upstreamable changes.

@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

servo-wpt-sync commented Feb 23, 2026

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#57973) title and body.

1 similar comment
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

servo-wpt-sync commented Feb 23, 2026

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#57973) title and body.

@jschwe
Copy link
Copy Markdown
Member

jschwe commented Feb 23, 2026

I like servo, or better known as its previous name servodriver to some other people.

I feel like the first sentence should be reworded, since servo is very overloaded and the first sentence is very confusing on its own.

jschwe
jschwe reviewed Feb 23, 2026
View reviewed changes
tests/wpt/tests/tools/wptrunner/wptrunner/browsers/servo.py
)
except requests.exceptions.ConnectionError:
self.logger.debug("Browser already shut down (connection refused)")
return
Copy link
Copy Markdown
Member

@jschwe jschwe Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I understand correctly this break -> return is the main improvement / fix, correct?

Copy link
Copy Markdown
Member Author

@yezhizhen yezhizhen Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Feb 23, 2026
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

servo-wpt-sync commented Feb 23, 2026

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#57973) title and body.

@yezhizhen
Copy link
Copy Markdown
Member Author

yezhizhen commented Feb 23, 2026

I like servo, or better known as its previous name servodriver to some other people.

I feel like the first sentence should be reworded, since servo is very overloaded and the first sentence is very confusing on its own.

I just added "runner" after "servo". What do you think?

@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

servo-wpt-sync commented Feb 23, 2026

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#57973) title and body.

@jschwe
Copy link
Copy Markdown
Member

jschwe commented Feb 23, 2026

@yezhizhen Perhaps adding webdriver to the title or first sentence would make the context clearer?

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 23, 2026
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

servo-wpt-sync commented Feb 23, 2026

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#57973).

@yezhizhen yezhizhen changed the title servo: Reduce recursive execution for shutdown servo harness: Reduce recursive execution for shutdown Feb 23, 2026
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

servo-wpt-sync commented Feb 23, 2026

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#57973) title and body.

@yezhizhen yezhizhen added this pull request to the merge queue Feb 23, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Feb 23, 2026
@yezhizhen
Copy link
Copy Markdown
Member Author

yezhizhen commented Feb 23, 2026

I compared a few action runs. This seems to reduce about 18 seconds per chunk in average. For 20 chunks, it would be ~360 seconds.

Merged via the queue into servo:main with commit c661dd0 Feb 23, 2026
34 checks passed
@yezhizhen yezhizhen deleted the servo-faster branch February 23, 2026 09:30
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Feb 23, 2026
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

servo-wpt-sync commented Feb 23, 2026

⛔ Failed to properly merge the upstream pull request (web-platform-tests/wpt#57973). Please address any CI issues and try to merge manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jschwe jschwe jschwe approved these changes

@xiaochengh xiaochengh Awaiting requested review from xiaochengh

Assignees

No one assigned

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@yezhizhen @servo-wpt-sync @jschwe @servo-highfive