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

Serve GZipped Static Assets #7788

Merged
merged 1 commit into from
May 21, 2024
Merged

Serve GZipped Static Assets #7788

merged 1 commit into from
May 21, 2024

Conversation

eladlachmi
Copy link
Contributor

Change Description

Added a Gzip handler to gzip files served by the embedded filesystem handler. This results in a ~63% reduction in transferred bytes.

Before:

Screenshot 2024-05-21 at 10 21 21

After:

Screenshot 2024-05-21 at 10 23 25

@eladlachmi eladlachmi added include-changelog PR description should be included in next release changelog minor-change Used for PRs that don't require issue attached labels May 21, 2024
@eladlachmi eladlachmi requested a review from a team May 21, 2024 07:44
@eladlachmi eladlachmi self-assigned this May 21, 2024
Copy link

E2E Test Results - DynamoDB Local - Local Block Adapter

13 passed

Copy link
Contributor

@arielshaqed arielshaqed left a comment

Choose a reason for hiding this comment

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

Approving since it seems better, but I'd like to avoid the overhead of repeatedly compressing some static content.

@@ -33,7 +34,8 @@ func NewUIHandler(gatewayDomains []string, snippets []params.CodeSnippet) http.H
panic(err)
}
fileSystem := http.FS(injectedContent)
etagHandler := EtagMiddleware(injectedContent, http.StripPrefix("/", http.FileServer(fileSystem)))
gzipHandler := gziphandler.GzipHandler(http.FileServer(fileSystem))
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure that I understand. We are serving just a few bundles, they are statically embedded into the executable. Can we hold the precompress the bundled objects and serve them directly but compressed? (Does any browser from the past 10 years not support this?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're not wrong. However, it's unusual to store GZipped assets on disk. GZipping the response is usually the responsibility of some reverse proxy in front of the asset server (e.g., nginx, Varnish, or something like CloudFront.)
I'll concede that most reasons for doing this aren't relevant to our use case. However, I decided to go this route under these assumptions:

  1. Low complexity
  2. Static assets can be cached client side and we can expect a low request rate in regular use
  3. The Go impl. isn't as efficient as the one used by nginx (C-based), but not far from it and we're using the default level, so this isn't expected to create any noticeable additional compute load

@eladlachmi eladlachmi merged commit 29e0022 into master May 21, 2024
39 checks passed
@eladlachmi eladlachmi deleted the serve-static-assets-gzipped branch May 21, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include-changelog PR description should be included in next release changelog minor-change Used for PRs that don't require issue attached
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants