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

Switch syntect regex engine over to onig and add newer JS syntax file #1944

Merged
merged 2 commits into from
Dec 6, 2022

Conversation

Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Dec 1, 2022

Changing the engine ~halves the time it takes to render readmes. Timings from rendering all readmes in the database dump from a couple years ago:

before: rendered 154040/154040 in 1.268288ms on average
after: rendered 154040/154040 in 583.846µs on average

It's also required to support the JavaScript (Babel) syntax file. This syntax doesn't run into the issue mentioned in #1942, but as I mention there I don't think this is going to be the only trigger so I want to see if there's something else easy we could do to avoid it completely.

(Draft because I'm still rebuilding my local server to actually check that the JS syntax looks good when rendered).

@github-actions github-actions bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Dec 1, 2022
@jsha
Copy link
Contributor

jsha commented Dec 2, 2022

What are the after timings across the database dump if you collect the worst-case runtime on any of the READMEs?

@syphar
Copy link
Member

syphar commented Dec 5, 2022

@Nemo157 @jyn514 is this ready to be merged?

I'm confused by the approval & the draft state of this PR :)

@Nemo157
Copy link
Member Author

Nemo157 commented Dec 5, 2022

The code is done, but I wanted to look at the actual UI before undrafting. I was having issues rebuilding the docker image (for some reason Ubuntu's update server was 403ing me), now that's been rebuilt but that also updated the database image which broke, so I need to figure out how to revert/migrate that sometime tonight.

docsrs-db-1  | 2022-12-05 10:25:14.779 UTC [1] FATAL:  database files are incompatible with server
docsrs-db-1  | 2022-12-05 10:25:14.779 UTC [1] DETAIL:  The data directory was initialized by PostgreSQL version 12, which is not compatible with this version 15.1.

@Nemo157 Nemo157 marked this pull request as ready for review December 5, 2022 23:09
@Nemo157
Copy link
Member Author

Nemo157 commented Dec 5, 2022

Got my local server running and check a couple JS files, they render just fine still.

@syphar syphar merged commit b39943e into rust-lang:master Dec 6, 2022
@github-actions github-actions bot added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Dec 6, 2022
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Dec 7, 2022
@Nemo157 Nemo157 deleted the new-js branch May 31, 2023 22:28
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.

4 participants