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

[performance] media processing improvements #1288

Conversation

NyaaaWhatsUpDoc
Copy link
Member

@NyaaaWhatsUpDoc NyaaaWhatsUpDoc commented Jan 2, 2023

Description

  • pulls in latest go-store and go-cache versions
  • wraps media encoding/decoding handling to be mostly contained within gtsImage{} and gtsVideo{} types for neatness
  • decreases JPEG thumb quality to 70% (it's only a thumb, helps with filesize)
  • uses memory pools for JPEG and PNG encoding
  • updates media processing to no longer use the atomic state variables (as these were mostly unused), all is now just done within mutex
  • general tidy-up of media processing which improves: neatness, memory usage, error handling
  • use larger write buffer size when streaming data into storage (now uses 16KiB, previously used 4KiB which is quite small)
  • fix presigned S3 URL cache call (fixes [bug] Panic probably related to S3 presigned URL caching #1279)
  • changes test filenames / paths to fit with updated changes
  • removes tee'd streaming of recache to both client and storage: in cases of slow client connections each tee stream could block a media worker pool function, which on instances with multiple users could very quickly exhaust available media workers
  • run a disk storage clean operation (removes empty folders) on storage open

Checklist

  • I/we have read the GoToSocial contribution guidelines.
  • I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
  • I/we have performed a self-review of added code.
  • I/we have written code that is legible and maintainable by others.
  • I/we have commented the added code, particularly in hard-to-understand areas.
  • I/we have made any necessary changes to documentation.
  • I/we have added tests that cover new code.
  • I/we have run tests and they pass locally with the changes.
  • I/we have run go fmt ./... and golangci-lint run.

@NyaaaWhatsUpDoc NyaaaWhatsUpDoc changed the title Performance/media processing improvements [performance] media processing improvements Jan 2, 2023
@tsmethurst
Copy link
Contributor

Still reviewing but it looks good so far. Does it make sense to try to include #1295 in this, or should that be a separate PR? Either is OK imo

@NyaaaWhatsUpDoc
Copy link
Member Author

Still reviewing but it looks good so far. Does it make sense to try to include #1295 in this, or should that be a separate PR? Either is OK imo

it might as well be included in this PR tbh :)

@NyaaaWhatsUpDoc NyaaaWhatsUpDoc force-pushed the performance/media-processing-improvements branch from e2f0d3c to b7371e0 Compare January 7, 2023 18:54
…ired syscalls

Signed-off-by: kim <grufwub@gmail.com>
… improved media processing AlreadyExists error handling

Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
@NyaaaWhatsUpDoc NyaaaWhatsUpDoc force-pushed the performance/media-processing-improvements branch from b7371e0 to 89df54b Compare January 10, 2023 18:22
Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
@NyaaaWhatsUpDoc NyaaaWhatsUpDoc marked this pull request as ready for review January 10, 2023 20:55
Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
Copy link
Contributor

@tsmethurst tsmethurst left a comment

Choose a reason for hiding this comment

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

Looks fucking great in general. I have a few small comments, but this seems more or less ready to merge, to me. Thanks for your hard work on this!! <3

internal/api/fileserver/servefile.go Outdated Show resolved Hide resolved
internal/media/processingemoji.go Show resolved Hide resolved
internal/media/processingmedia.go Show resolved Hide resolved
internal/media/types.go Show resolved Hide resolved
internal/processing/media/getfile.go Show resolved Hide resolved
Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
@tsmethurst tsmethurst merged commit 5318054 into superseriousbusiness:main Jan 11, 2023
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.

[bug] Panic probably related to S3 presigned URL caching
3 participants