puffin icon indicating copy to clipboard operation
puffin copied to clipboard

Request for Comments: puffin_http: use async local task

Open gwen-lg opened this issue 3 years ago • 4 comments

Checklist

  • [x] I have read the Contributor Guide
  • [x] I have read and agree to the Code of Conduct
  • [x] I have added a description of my changes and why I'd like them included in the section below

Description of Changes

Use async Rust to manage puffin server (puffin_http) tasks. An regroup it in one dedicated thread. I think it's proper

Changes are not totally clean, but before improve it, I like to know if your are interested by this approach.

I have tested this change with my dedicated app : https://github.com/gwen-lg/puffin-test-app, but I think it's need test with real application/running game.

Also I have identified this think to do before merge it :

  • tried to have task name management
  • clean commented code
  • remove comment of crossbeam_channel
  • finish rewrite of architecture section of puffin_http/readme.md

Related Issues

This change are not related to a github issue. But this change permit to implement

A captures of result : image image

gwen-lg avatar Aug 16 '22 12:08 gwen-lg

This adds A LOT of dependencies, which will greatly increase compile times for any user of puffin_http.

What concrete benefits does this PR bring to justify that?

emilk avatar Aug 25 '22 06:08 emilk

Sorry, I have forgotten this part in pull request.

I have stared this after have tried to use puffin with app without frame (like a cooking process). With some test, I wasn't able to get first frame (loading) of an application and no more have a capture for and application without frame (simulated with a one frame application). This change allow :

  • to a client to connect without the end of the first frame.
  • to get the first frames, event when they are short.
  • add a functionality to wait connection of a client before continue app running (ex: wait_client_in_server )
  • get markers for "one run application" (application without frame)

PS: I have tested this with this : puffin-test-app

gwen-lg avatar Aug 25 '22 10:08 gwen-lg

I was thinking of the possibility of enable async with feature option. This would be ideal for users, if there already have async code or if they want, they could enable async management. But it's implies a lot of code duplicate.

What did you think of it?

gwen-lg avatar Sep 29 '22 20:09 gwen-lg

@gwen-lg thanks for submitting the PR, sorry for the lack of activity, I will get back to the questions in this PR. I can try it for our internal project and see how it behaves and let you know if there is interest in continuing with the PR.

TimonPost avatar Oct 10 '22 20:10 TimonPost