Skip to content

Conversation

@lukewilliamboswell
Copy link
Collaborator

No description provided.

@lukewilliamboswell lukewilliamboswell changed the title [WIP] Upgrade to purity inference Upgrade to purity inference Dec 19, 2024
@lukewilliamboswell
Copy link
Collaborator Author

Down to one failing test... currently being skipped to confirm CI is running to completion.

@lukewilliamboswell lukewilliamboswell marked this pull request as ready for review December 19, 2024 03:50
@@ -1,13 +0,0 @@
module [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we removing Sleep and the related example? Is this temporary for getting everything working? It seems to be in basic-cli, so maybe it got missed in the great copy of `87?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The way it was implemented it was putting the whole thread to sleep. I think I added it mindlessly porting across from basic-cli... but it's not something I think we want in a multithreaded HTTP web server, at least not like this.

Copy link
Member

Choose a reason for hiding this comment

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

basic webserver spawns a thread per request. So sleep is actually fine (though silly to do).

Copy link
Collaborator

Choose a reason for hiding this comment

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

This image is tiny at 152 bytes, I agree that including it is better than generating it every time

Comment on lines +75 to +87
to_host_method : Method -> _
to_host_method = \method ->
when method is
OPTIONS -> 5
GET -> 3
POST -> 7
PUT -> 8
DELETE -> 1
HEAD -> 4
TRACE -> 9
CONNECT -> 0
PATCH -> 6
EXTENSION _ -> 2
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did have a proper rust Method impl, but we were having segafults during the development of this upgrade and suspected the Method to be an issue (possibly alignment or refcounting). Turned out not to be the source of our issues, but I decided it was easier just to leave it like this and not re-generate glue.

The delta is the space difference between a u8 and u64 per request.

Copy link
Member

Choose a reason for hiding this comment

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

Due to padding I think there literally is no difference.

@lukewilliamboswell lukewilliamboswell merged commit 0caba41 into main Dec 19, 2024
4 checks passed
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