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

Micro-optimization: set cache header on kernelspecs logos #702

Closed
mlucool opened this issue Feb 24, 2022 · 2 comments
Closed

Micro-optimization: set cache header on kernelspecs logos #702

mlucool opened this issue Feb 24, 2022 · 2 comments

Comments

@mlucool
Copy link

mlucool commented Feb 24, 2022

Problem

kernelspecs's often contain png logos. These logos are on the order of 3kb each. Since kernels can't share a logo, each kernel contains a copy of the logo it provides. This leads to 3*number of kernels of uncached content being loaded on each page. I have ~20 kernels today, so that's 60kb being sent per refresh. (Note: this was flagged in lighthouse).

Proposed Solution

Ideally, we'd be able to share generic logos, but since that is a bigger change, I propose we add long cache headers on logos served from kernelspecs/<kernel>/logo-64x64.png. Unlike the kernelspec itself, these are expected to very rarely change.

@kevin-bates
Copy link
Member

Hi @mlucool - this seems like a great idea. I don't know what is involved to do this, but I thought I'd point out the handler responsible for returning the logos (resources) in case you, or anyone else, would like to contribute the changes:

class KernelSpecResourceHandler(web.StaticFileHandler, JupyterHandler):
SUPPORTED_METHODS = ("GET", "HEAD")
auth_resource = AUTH_RESOURCE
def initialize(self):
web.StaticFileHandler.initialize(self, path="")
@web.authenticated
@authorized
def get(self, kernel_name, path, include_body=True):
ksm = self.kernel_spec_manager
try:
self.root = ksm.get_kernel_spec(kernel_name).resource_dir
except KeyError as e:
raise web.HTTPError(404, "Kernel spec %s not found" % kernel_name) from e
self.log.debug("Serving kernel resource from: %s", self.root)
return web.StaticFileHandler.get(self, path, include_body=include_body)
@web.authenticated
@authorized
def head(self, kernel_name, path):
return self.get(kernel_name, path, include_body=False)

@mlucool
Copy link
Author

mlucool commented Feb 24, 2022

Humm, it seems the code you pointed to already uses a StaticFileHandler which sets an ETag. Lighthouse is upset that there is not Cache TTL, which should be easy enough to change.

image

divyansshhh added a commit to divyansshhh/jupyter_server that referenced this issue Mar 26, 2022
divyansshhh added a commit to divyansshhh/jupyter_server that referenced this issue Mar 26, 2022
@mlucool mlucool closed this as completed Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants