mirror of
				https://github.com/superseriousbusiness/gotosocial.git
				synced 2025-10-28 13:52:25 -05:00 
			
		
		
		
	[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 <grufwub@gmail.com> Co-committed-by: kim <grufwub@gmail.com>
This commit is contained in:
		
					parent
					
						
							
								baf2c54730
							
						
					
				
			
			
				commit
				
					
						5fd52369c9
					
				
			
		
					 5 changed files with 62 additions and 21 deletions
				
			
		|  | @ -51,6 +51,7 @@ func (d *Dereferencer) GetEmoji( | ||||||
| 	remoteURL string, | 	remoteURL string, | ||||||
| 	info media.AdditionalEmojiInfo, | 	info media.AdditionalEmojiInfo, | ||||||
| 	refresh bool, | 	refresh bool, | ||||||
|  | 	async bool, | ||||||
| ) ( | ) ( | ||||||
| 	*gtsmodel.Emoji, | 	*gtsmodel.Emoji, | ||||||
| 	error, | 	error, | ||||||
|  | @ -66,7 +67,7 @@ func (d *Dereferencer) GetEmoji( | ||||||
| 
 | 
 | ||||||
| 	if emoji != nil { | 	if emoji != nil { | ||||||
| 		// This was an existing emoji, pass to refresh func. | 		// 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 == "" { | 	if domain == "" { | ||||||
|  | @ -112,6 +113,7 @@ func (d *Dereferencer) GetEmoji( | ||||||
| 				info, | 				info, | ||||||
| 			) | 			) | ||||||
| 		}, | 		}, | ||||||
|  | 		async, | ||||||
| 	) | 	) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | @ -130,6 +132,7 @@ func (d *Dereferencer) RefreshEmoji( | ||||||
| 	emoji *gtsmodel.Emoji, | 	emoji *gtsmodel.Emoji, | ||||||
| 	info media.AdditionalEmojiInfo, | 	info media.AdditionalEmojiInfo, | ||||||
| 	force bool, | 	force bool, | ||||||
|  | 	async bool, | ||||||
| ) ( | ) ( | ||||||
| 	*gtsmodel.Emoji, | 	*gtsmodel.Emoji, | ||||||
| 	error, | 	error, | ||||||
|  | @ -162,7 +165,7 @@ func (d *Dereferencer) RefreshEmoji( | ||||||
| 		// We still want to make sure | 		// We still want to make sure | ||||||
| 		// the emoji is cached. Simply | 		// the emoji is cached. Simply | ||||||
| 		// check whether emoji is cached. | 		// check whether emoji is cached. | ||||||
| 		return d.RecacheEmoji(ctx, emoji) | 		return d.RecacheEmoji(ctx, emoji, async) | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	// Can't refresh local. | 	// Can't refresh local. | ||||||
|  | @ -207,6 +210,7 @@ func (d *Dereferencer) RefreshEmoji( | ||||||
| 				info, | 				info, | ||||||
| 			) | 			) | ||||||
| 		}, | 		}, | ||||||
|  | 		async, | ||||||
| 	) | 	) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | @ -222,6 +226,7 @@ func (d *Dereferencer) RefreshEmoji( | ||||||
| func (d *Dereferencer) RecacheEmoji( | func (d *Dereferencer) RecacheEmoji( | ||||||
| 	ctx context.Context, | 	ctx context.Context, | ||||||
| 	emoji *gtsmodel.Emoji, | 	emoji *gtsmodel.Emoji, | ||||||
|  | 	async bool, | ||||||
| ) ( | ) ( | ||||||
| 	*gtsmodel.Emoji, | 	*gtsmodel.Emoji, | ||||||
| 	error, | 	error, | ||||||
|  | @ -272,23 +277,24 @@ func (d *Dereferencer) RecacheEmoji( | ||||||
| 				data, | 				data, | ||||||
| 			) | 			) | ||||||
| 		}, | 		}, | ||||||
|  | 		async, | ||||||
| 	) | 	) | ||||||
| 
 |  | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| // processingEmojiSafely provides concurrency-safe processing of | // processingEmojiSafely provides concurrency-safe processing of | ||||||
| // an emoji with given shortcode+domain. if a copy of the emoji is | // an emoji with given shortcode+domain. if a copy of the emoji is | ||||||
| // not already being processed, the given 'process' callback will | // 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( | func (d *Dereferencer) processEmojiSafely( | ||||||
| 	ctx context.Context, | 	ctx context.Context, | ||||||
| 	shortcodeDomain string, | 	shortcodeDomain string, | ||||||
| 	process func() (*media.ProcessingEmoji, error), | 	process func() (*media.ProcessingEmoji, error), | ||||||
|  | 	async bool, | ||||||
| ) ( | ) ( | ||||||
| 	emoji *gtsmodel.Emoji, | 	emoji *gtsmodel.Emoji, | ||||||
| 	err error, | 	err error, | ||||||
| ) { | ) { | ||||||
| 
 |  | ||||||
| 	// Acquire map lock. | 	// Acquire map lock. | ||||||
| 	d.derefEmojisMu.Lock() | 	d.derefEmojisMu.Lock() | ||||||
| 
 | 
 | ||||||
|  | @ -309,25 +315,34 @@ func (d *Dereferencer) processEmojiSafely( | ||||||
| 
 | 
 | ||||||
| 		// Add processing emoji media to hash map. | 		// Add processing emoji media to hash map. | ||||||
| 		d.derefEmojis[shortcodeDomain] = processing | 		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() { | 		defer func() { | ||||||
| 			// Remove on finish. | 			// Remove on finish. | ||||||
| 			d.derefEmojisMu.Lock() | 			d.derefEmojisMu.Lock() | ||||||
| 			delete(d.derefEmojis, shortcodeDomain) | 			delete(d.derefEmojis, shortcodeDomain) | ||||||
| 			d.derefEmojisMu.Unlock() | 			d.derefEmojisMu.Unlock() | ||||||
| 		}() | 		}() | ||||||
| 	} |  | ||||||
| 
 | 
 | ||||||
| 	// Unlock map. | 		// Perform emoji load operation. | ||||||
| 	unlock() | 		emoji, err = processing.Load(ctx) | ||||||
|  | 		if err != nil { | ||||||
|  | 			err = gtserror.Newf("error loading emoji %s: %w", shortcodeDomain, err) | ||||||
| 
 | 
 | ||||||
| 	// Perform emoji load operation. | 			// TODO: in time we should return checkable flags by gtserror.Is___() | ||||||
| 	emoji, err = processing.Load(ctx) | 			// which can determine if loading error should allow remaining placeholder. | ||||||
| 	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. |  | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	return | 	return | ||||||
|  | @ -364,7 +379,7 @@ func (d *Dereferencer) fetchEmojis( | ||||||
| 				URI:                  &placeholder.URI, | 				URI:                  &placeholder.URI, | ||||||
| 				ImageRemoteURL:       &placeholder.ImageRemoteURL, | 				ImageRemoteURL:       &placeholder.ImageRemoteURL, | ||||||
| 				ImageStaticRemoteURL: &placeholder.ImageStaticRemoteURL, | 				ImageStaticRemoteURL: &placeholder.ImageStaticRemoteURL, | ||||||
| 			}, force) | 			}, force, true) | ||||||
| 			if err != nil { | 			if err != nil { | ||||||
| 				log.Errorf(ctx, "error refreshing emoji: %v", err) | 				log.Errorf(ctx, "error refreshing emoji: %v", err) | ||||||
| 
 | 
 | ||||||
|  | @ -396,6 +411,7 @@ func (d *Dereferencer) fetchEmojis( | ||||||
| 				ImageStaticRemoteURL: &placeholder.ImageStaticRemoteURL, | 				ImageStaticRemoteURL: &placeholder.ImageStaticRemoteURL, | ||||||
| 			}, | 			}, | ||||||
| 			false, | 			false, | ||||||
|  | 			true, | ||||||
| 		) | 		) | ||||||
| 		if err != nil { | 		if err != nil { | ||||||
| 			if emoji == nil { | 			if emoji == nil { | ||||||
|  |  | ||||||
|  | @ -54,6 +54,7 @@ func (suite *EmojiTestSuite) TestDereferenceEmojiBlocking() { | ||||||
| 			VisibleInPicker:      &emojiVisibleInPicker, | 			VisibleInPicker:      &emojiVisibleInPicker, | ||||||
| 		}, | 		}, | ||||||
| 		false, | 		false, | ||||||
|  | 		false, | ||||||
| 	) | 	) | ||||||
| 	suite.NoError(err) | 	suite.NoError(err) | ||||||
| 	suite.NotNil(emoji) | 	suite.NotNil(emoji) | ||||||
|  |  | ||||||
|  | @ -44,11 +44,6 @@ type ProcessingEmoji struct { | ||||||
| 	mgr       *Manager          // mgr instance (access to db / storage) | 	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. | // 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) { | func (p *ProcessingEmoji) Load(ctx context.Context) (*gtsmodel.Emoji, error) { | ||||||
| 	emoji, done, err := p.load(ctx) | 	emoji, done, err := p.load(ctx) | ||||||
|  | @ -63,6 +58,33 @@ func (p *ProcessingEmoji) Load(ctx context.Context) (*gtsmodel.Emoji, error) { | ||||||
| 	return emoji, err | 	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. | // load is the package private form of load() that is wrapped to catch context canceled. | ||||||
| func (p *ProcessingEmoji) load(ctx context.Context) ( | func (p *ProcessingEmoji) load(ctx context.Context) ( | ||||||
| 	emoji *gtsmodel.Emoji, | 	emoji *gtsmodel.Emoji, | ||||||
|  |  | ||||||
|  | @ -293,6 +293,7 @@ func (p *Processor) emojiUpdateCopy( | ||||||
| 	// Ensure target emoji is locally cached. | 	// Ensure target emoji is locally cached. | ||||||
| 	target, err := p.federator.RecacheEmoji(ctx, | 	target, err := p.federator.RecacheEmoji(ctx, | ||||||
| 		target, | 		target, | ||||||
|  | 		false, | ||||||
| 	) | 	) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		err := gtserror.Newf("error recaching emoji %s: %w", target.ImageRemoteURL, err) | 		err := gtserror.Newf("error recaching emoji %s: %w", target.ImageRemoteURL, err) | ||||||
|  |  | ||||||
|  | @ -247,6 +247,7 @@ func (p *Processor) getEmojiContent( | ||||||
| 	emoji, err = p.federator.RecacheEmoji( | 	emoji, err = p.federator.RecacheEmoji( | ||||||
| 		ctx, | 		ctx, | ||||||
| 		emoji, | 		emoji, | ||||||
|  | 		false, | ||||||
| 	) | 	) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		err := gtserror.Newf("error recaching emoji: %w", err) | 		err := gtserror.Newf("error recaching emoji: %w", err) | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue