From 5fd52369c98eb83b735533674bcc4c3b227f69d2 Mon Sep 17 00:00:00 2001 From: kim Date: Wed, 8 Oct 2025 14:13:40 +0200 Subject: [PATCH] [performance] handle emoji refreshes asynchronously when fetched as part of account|status dereferences (#4486) # Description Updates our dereferencer emoji handling to work asynchronously when going through the route of account or status dereferencing. closes https://codeberg.org/superseriousbusiness/gotosocial/issues/4485 ## Checklist - [x] I/we have read the [GoToSocial contribution guidelines](https://codeberg.org/superseriousbusiness/gotosocial/src/branch/main/CONTRIBUTING.md). - [x] I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat. - [x] I/we have not leveraged AI to create the proposed changes. - [x] I/we have performed a self-review of added code. - [x] 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. - [x] I/we have run `go fmt ./...` and `golangci-lint run`. Reviewed-on: https://codeberg.org/superseriousbusiness/gotosocial/pulls/4486 Co-authored-by: kim Co-committed-by: kim --- internal/federation/dereferencing/emoji.go | 48 ++++++++++++------- .../federation/dereferencing/emoji_test.go | 1 + internal/media/processingemoji.go | 32 +++++++++++-- internal/processing/admin/emoji.go | 1 + internal/processing/media/getfile.go | 1 + 5 files changed, 62 insertions(+), 21 deletions(-) diff --git a/internal/federation/dereferencing/emoji.go b/internal/federation/dereferencing/emoji.go index fbe2e4d98..b8ecfa6e1 100644 --- a/internal/federation/dereferencing/emoji.go +++ b/internal/federation/dereferencing/emoji.go @@ -51,6 +51,7 @@ func (d *Dereferencer) GetEmoji( remoteURL string, info media.AdditionalEmojiInfo, refresh bool, + async bool, ) ( *gtsmodel.Emoji, error, @@ -66,7 +67,7 @@ func (d *Dereferencer) GetEmoji( if emoji != nil { // This was an existing emoji, pass to refresh func. - return d.RefreshEmoji(ctx, emoji, info, refresh) + return d.RefreshEmoji(ctx, emoji, info, refresh, async) } if domain == "" { @@ -112,6 +113,7 @@ func (d *Dereferencer) GetEmoji( info, ) }, + async, ) } @@ -130,6 +132,7 @@ func (d *Dereferencer) RefreshEmoji( emoji *gtsmodel.Emoji, info media.AdditionalEmojiInfo, force bool, + async bool, ) ( *gtsmodel.Emoji, error, @@ -162,7 +165,7 @@ func (d *Dereferencer) RefreshEmoji( // We still want to make sure // the emoji is cached. Simply // check whether emoji is cached. - return d.RecacheEmoji(ctx, emoji) + return d.RecacheEmoji(ctx, emoji, async) } // Can't refresh local. @@ -207,6 +210,7 @@ func (d *Dereferencer) RefreshEmoji( info, ) }, + async, ) } @@ -222,6 +226,7 @@ func (d *Dereferencer) RefreshEmoji( func (d *Dereferencer) RecacheEmoji( ctx context.Context, emoji *gtsmodel.Emoji, + async bool, ) ( *gtsmodel.Emoji, error, @@ -272,23 +277,24 @@ func (d *Dereferencer) RecacheEmoji( data, ) }, + async, ) - } // processingEmojiSafely provides concurrency-safe processing of // an emoji with given shortcode+domain. if a copy of the emoji is // not already being processed, the given 'process' callback will -// be used to generate new *media.ProcessingEmoji{} instance. +// be used to generate new *media.ProcessingEmoji{} instance. async +// determines whether to load it immediately, or in the background. func (d *Dereferencer) processEmojiSafely( ctx context.Context, shortcodeDomain string, process func() (*media.ProcessingEmoji, error), + async bool, ) ( emoji *gtsmodel.Emoji, err error, ) { - // Acquire map lock. d.derefEmojisMu.Lock() @@ -309,25 +315,34 @@ func (d *Dereferencer) processEmojiSafely( // Add processing emoji media to hash map. d.derefEmojis[shortcodeDomain] = processing + } + // Unlock map. + unlock() + + if async { + emoji = processing.LoadAsync(func() { + // Remove on finish. + d.derefEmojisMu.Lock() + delete(d.derefEmojis, shortcodeDomain) + d.derefEmojisMu.Unlock() + }) + } else { defer func() { // Remove on finish. d.derefEmojisMu.Lock() delete(d.derefEmojis, shortcodeDomain) d.derefEmojisMu.Unlock() }() - } - // Unlock map. - unlock() + // Perform emoji load operation. + emoji, err = processing.Load(ctx) + if err != nil { + err = gtserror.Newf("error loading emoji %s: %w", shortcodeDomain, err) - // Perform emoji load operation. - emoji, err = processing.Load(ctx) - if err != nil { - err = gtserror.Newf("error loading emoji %s: %w", shortcodeDomain, err) - - // TODO: in time we should return checkable flags by gtserror.Is___() - // which can determine if loading error should allow remaining placeholder. + // TODO: in time we should return checkable flags by gtserror.Is___() + // which can determine if loading error should allow remaining placeholder. + } } return @@ -364,7 +379,7 @@ func (d *Dereferencer) fetchEmojis( URI: &placeholder.URI, ImageRemoteURL: &placeholder.ImageRemoteURL, ImageStaticRemoteURL: &placeholder.ImageStaticRemoteURL, - }, force) + }, force, true) if err != nil { log.Errorf(ctx, "error refreshing emoji: %v", err) @@ -396,6 +411,7 @@ func (d *Dereferencer) fetchEmojis( ImageStaticRemoteURL: &placeholder.ImageStaticRemoteURL, }, false, + true, ) if err != nil { if emoji == nil { diff --git a/internal/federation/dereferencing/emoji_test.go b/internal/federation/dereferencing/emoji_test.go index c8c905781..5a4a1f601 100644 --- a/internal/federation/dereferencing/emoji_test.go +++ b/internal/federation/dereferencing/emoji_test.go @@ -54,6 +54,7 @@ func (suite *EmojiTestSuite) TestDereferenceEmojiBlocking() { VisibleInPicker: &emojiVisibleInPicker, }, false, + false, ) suite.NoError(err) suite.NotNil(emoji) diff --git a/internal/media/processingemoji.go b/internal/media/processingemoji.go index d28edcc0c..feb54da2f 100644 --- a/internal/media/processingemoji.go +++ b/internal/media/processingemoji.go @@ -44,11 +44,6 @@ type ProcessingEmoji struct { mgr *Manager // mgr instance (access to db / storage) } -// ID returns the ID of the underlying emoji. -func (p *ProcessingEmoji) ID() string { - return p.emoji.ID // immutable, safe outside mutex. -} - // LoadEmoji blocks until the static and fullsize image has been processed, and then returns the completed emoji. func (p *ProcessingEmoji) Load(ctx context.Context) (*gtsmodel.Emoji, error) { emoji, done, err := p.load(ctx) @@ -63,6 +58,33 @@ func (p *ProcessingEmoji) Load(ctx context.Context) (*gtsmodel.Emoji, error) { return emoji, err } +func (p *ProcessingEmoji) LoadAsync(deferred func()) *gtsmodel.Emoji { + p.mgr.state.Workers.Dereference.Queue.Push(func(ctx context.Context) { + if deferred != nil { + defer deferred() + } + + if _, _, err := p.load(ctx); err != nil { + log.Errorf(ctx, "error loading emoji: %v", err) + } + }) + + // Placeholder returns a copy of internally stored processing placeholder, + // returning only the fields that may be known *before* completion, + // and as such all fields which are safe to concurrently read. + placeholder := new(gtsmodel.Emoji) + placeholder.ID = p.emoji.ID + placeholder.Shortcode = p.emoji.Shortcode + placeholder.Domain = p.emoji.Domain + placeholder.Cached = new(bool) + placeholder.ImageRemoteURL = p.emoji.ImageRemoteURL + placeholder.ImageStaticRemoteURL = p.emoji.ImageStaticRemoteURL + placeholder.Disabled = p.emoji.Disabled + placeholder.VisibleInPicker = p.emoji.VisibleInPicker + placeholder.CategoryID = p.emoji.CategoryID + return placeholder +} + // load is the package private form of load() that is wrapped to catch context canceled. func (p *ProcessingEmoji) load(ctx context.Context) ( emoji *gtsmodel.Emoji, diff --git a/internal/processing/admin/emoji.go b/internal/processing/admin/emoji.go index b45d31e96..5c391bf82 100644 --- a/internal/processing/admin/emoji.go +++ b/internal/processing/admin/emoji.go @@ -293,6 +293,7 @@ func (p *Processor) emojiUpdateCopy( // Ensure target emoji is locally cached. target, err := p.federator.RecacheEmoji(ctx, target, + false, ) if err != nil { err := gtserror.Newf("error recaching emoji %s: %w", target.ImageRemoteURL, err) diff --git a/internal/processing/media/getfile.go b/internal/processing/media/getfile.go index 2f46d6e0c..79ab08291 100644 --- a/internal/processing/media/getfile.go +++ b/internal/processing/media/getfile.go @@ -247,6 +247,7 @@ func (p *Processor) getEmojiContent( emoji, err = p.federator.RecacheEmoji( ctx, emoji, + false, ) if err != nil { err := gtserror.Newf("error recaching emoji: %w", err)