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

Reduce Data Flow #13025

Closed
kevinjhang opened this issue Aug 10, 2019 · 7 comments
Closed

Reduce Data Flow #13025

kevinjhang opened this issue Aug 10, 2019 · 7 comments
Labels
affected-medium This issue impacts approximately half of our customers area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-blazor-server ✔️ Resolution: Duplicate Resolved as a duplicate of another issue Pillar: Technical Debt reevaluate We need to reevaluate the issue and make a decision about it severity-nice-to-have This label is used by an internal tool Status: Resolved

Comments

@kevinjhang
Copy link

Is your feature request related to a problem? Please describe.

I am trying to do reduce data flow

Describe the solution you'd like

Rename "JS.RenderBatch" to "JS.RB"

Additional context

2019-08-10 184143

@kevinjhang
Copy link
Author

Maybe rename to "R" is better, It's shorter

@mkArtakMSFT mkArtakMSFT added the area-blazor Includes: Blazor, Razor Components label Aug 12, 2019
@mkArtakMSFT
Copy link
Member

Thanks for contacting us, @kevinjhang.
We've moved this issue to the Backlog milestone. This means that it is not going to happen for the coming release. We will reassess the backlog following the current release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources.

@mkArtakMSFT mkArtakMSFT added this to the Backlog milestone Aug 12, 2019
@mkArtakMSFT mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Aug 12, 2019
@ghost
Copy link

ghost commented Dec 13, 2019

I'd also like to chime in here. Forgive me if I'm incorrect, but monitoring the websocket messages received, it appears that a large amount of white-space is included in the message payload. I don't know if IIS optimizes this out or not, but I thought I'd ask for clarification.
image

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Oct 14, 2020

Blazor Server's renderbatch wire format is already pretty heavily optimized. You can find the implementation at https://github.com/dotnet/aspnetcore/blob/master/src/Components/Server/src/Circuits/RenderBatchWriter.cs. Note that we're not only optimizing for wire size, but also for efficiency of decoding on the client side, which is why it's highly valuable to pad each frame to the same length.

If you're interested in submitting a PR to optimize this further we'd certainly be interested.

Rename "JS.RenderBatch" to "JS.RB"

We'd need a stronger, more quantified justification in order to do that. Do you have a reasonable application that returns large renderbatches in which this change would make more than (say) 1% of difference? I'd be surprised if, in large renderbatches which need to be optimized, this change would have any noticeable effect.

Thanks for the suggestion though!

@SteveSandersonMS SteveSandersonMS added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Oct 14, 2020
@SteveSandersonMS SteveSandersonMS added affected-medium This issue impacts approximately half of our customers severity-nice-to-have This label is used by an internal tool labels Oct 14, 2020 — with ASP.NET Core Issue Ranking
@kevinjhang
Copy link
Author

TCPStream-1.txt (32,782 bytes)
TCPStream-2.txt (31,513 bytes)
TCPStream-2.txt = TCPStream-1.txt.Replace("RenderBatch","RB")
difference(%) = (32782-31513) / 32782 * 100% = 3.871%

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Oct 15, 2020
@SteveSandersonMS
Copy link
Member

@kevinjhang Thanks

@SteveSandersonMS SteveSandersonMS removed the Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. label Oct 15, 2020
@javiercn javiercn added feature-blazor-server reevaluate We need to reevaluate the issue and make a decision about it labels Apr 19, 2021
@mkArtakMSFT mkArtakMSFT modified the milestones: Backlog, BlazorPlanning Nov 5, 2023
@mkArtakMSFT
Copy link
Member

Closing this as #35897 is going to give us more benefits and basically obsolete this specific ask.

@mkArtakMSFT mkArtakMSFT closed this as not planned Won't fix, can't repro, duplicate, stale Dec 13, 2023
@mkArtakMSFT mkArtakMSFT added the ✔️ Resolution: Duplicate Resolved as a duplicate of another issue label Dec 13, 2023
@ghost ghost added the Status: Resolved label Dec 13, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affected-medium This issue impacts approximately half of our customers area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-blazor-server ✔️ Resolution: Duplicate Resolved as a duplicate of another issue Pillar: Technical Debt reevaluate We need to reevaluate the issue and make a decision about it severity-nice-to-have This label is used by an internal tool Status: Resolved
Projects
None yet
Development

No branches or pull requests

4 participants