Use shared_ptr for SGUIImageEffects to avoid both copy and delete
ClosedPublic
Actions

Authored by elexis on Aug 17 2019, 1:39 PM.

Details

Reviewers: None
Commits: rP22682: Use shared_ptr to avoid copying and deleting SGUIImageEffects, 2.5x faster in a…

Summary

While examining the entire GUI for useless (especially implicit/obfuscated) copies by adding the NONCOPYABLE keyword everywhere, I found this copy and deletion in rP1525.
The code looks like a primary example of a use case for a shared_ptr, because the thing gets created once, then assigned to N users, then the users disappear sometime later.
With a shared_ptr we don't have to care about erasure, when the last user is gone, the thing gets deleted.
With raw pointers, one wont be able to tell who has the responsibility to delete, so the current code just copies it.
I measured the performance by doing a benchmark and it is indeed faster to use the shared_ptr, 2.5 times faster.
Also if we look at the data structures, the effect has one bool and two colors, each of which have 4 floats,
whereas the shared_ptr only has the pointer and the reference counter afaik.
So all gathered evidence indicates that this is better off with a shared_ptr.

Test Plan

I have used the following patch to measure the performance. It rules out compiler optimizations by looping (so if it would elide copies, then the number wouldnt go up proportionally with the call count).
Notice that constant references work in CGUI.cpp, but these effects are discarded and not part of the CGUI "databases", because they are defined within the sprite.
For example

	<sprite name="ModernButtonRedOver">
		<effect add_color="60 42 42 0"/>
		<image texture="global/modern/button/red-unselected-left-top.png"
		...

Here the patch:

SGUIImageEffects_smart_copy_test.patch5 KBDownload

Here the performance results:

Smart pointer: 5
Smart pointer: 1
Smart pointer: 5
Smart pointer: 55
Smart pointer: 557
Smart pointer: 5591
Smart pointer: 48833
Copy: 1
Copy: 2
Copy: 19
Copy: 130
Copy: 1639
Copy: 14460
Copy: 128543
Smart is 2.632298 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 48
Smart pointer: 477
Smart pointer: 4780
Smart pointer: 47855
Copy: 0
Copy: 1
Copy: 15
Copy: 131
Copy: 1227
Copy: 12441
Copy: 124869
Smart is 2.609320 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4785
Smart pointer: 47820
Copy: 4
Copy: 1
Copy: 13
Copy: 126
Copy: 1239
Copy: 12617
Copy: 126083
Smart is 2.636616 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4779
Smart pointer: 47821
Copy: 0
Copy: 1
Copy: 12
Copy: 123
Copy: 1216
Copy: 12552
Copy: 125597
Smart is 2.626398 times faster
Smart pointer: 0
Smart pointer: 1
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4787
Smart pointer: 47834
Copy: 0
Copy: 1
Copy: 13
Copy: 135
Copy: 1268
Copy: 12515
Copy: 125927
Smart is 2.632584 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 483
Smart pointer: 4782
Smart pointer: 47810
Copy: 0
Copy: 1
Copy: 12
Copy: 124
Copy: 1223
Copy: 12986
Copy: 126226
Smart is 2.640159 times faster
Smart pointer: 0
Smart pointer: 1
Smart pointer: 5
Smart pointer: 48
Smart pointer: 478
Smart pointer: 4779
Smart pointer: 47825
Copy: 0
Copy: 1
Copy: 12
Copy: 126
Copy: 1233
Copy: 12449
Copy: 127364
Smart is 2.663126 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 48
Smart pointer: 477
Smart pointer: 4788
Smart pointer: 48092
Copy: 0
Copy: 1
Copy: 13
Copy: 127
Copy: 1244
Copy: 12299
Copy: 123143
Smart is 2.560571 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4786
Smart pointer: 47808
Copy: 0
Copy: 1
Copy: 15
Copy: 132
Copy: 1282
Copy: 12981
Copy: 124547
Smart is 2.605150 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4790
Smart pointer: 47794
Copy: 0
Copy: 1
Copy: 14
Copy: 125
Copy: 1220
Copy: 12460
Copy: 125262
Smart is 2.620873 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4778
Smart pointer: 47788
Copy: 0
Copy: 1
Copy: 12
Copy: 123
Copy: 1208
Copy: 12456
Copy: 122799
Smart is 2.569662 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 55
Smart pointer: 477
Smart pointer: 4778
Smart pointer: 47787
Copy: 4
Copy: 1
Copy: 13
Copy: 135
Copy: 1249
Copy: 12400
Copy: 124099
Smart is 2.596920 times faster
TIMER| common/modern/sprites.xml: 2.3177 s
TIMER| common/setup.xml: 61.6256 ms
Smart pointer: 1
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4897
Smart pointer: 48367
Copy: 0
Copy: 1
Copy: 15
Copy: 131
Copy: 1214
Copy: 12616
Copy: 125555
Smart is 2.595881 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4891
Smart pointer: 48349
Copy: 0
Copy: 1
Copy: 14
Copy: 125
Copy: 1236
Copy: 12932
Copy: 127010
Smart is 2.626942 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4856
Smart pointer: 48335
Copy: 0
Copy: 1
Copy: 17
Copy: 122
Copy: 1285
Copy: 12825
Copy: 128111
Smart is 2.650481 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4863
Smart pointer: 48361
Copy: 0
Copy: 1
Copy: 13
Copy: 122
Copy: 1227
Copy: 12938
Copy: 128614
Smart is 2.659457 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4945
Smart pointer: 48364
Copy: 0
Copy: 7
Copy: 12
Copy: 124
Copy: 1179
Copy: 12409
Copy: 124868
Smart is 2.581838 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4865
Smart pointer: 48359
Copy: 0
Copy: 6
Copy: 11
Copy: 127
Copy: 1275
Copy: 12323
Copy: 124221
Smart is 2.568726 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 5
Smart pointer: 48
Smart pointer: 477
Smart pointer: 4901
Smart pointer: 48345
Copy: 0
Copy: 1
Copy: 14
Copy: 124
Copy: 1236
Copy: 12638
Copy: 124854
Smart is 2.582563 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4857
Smart pointer: 48383
Copy: 0
Copy: 1
Copy: 17
Copy: 135
Copy: 1270
Copy: 13152
Copy: 124636
Smart is 2.576029 times faster
Smart pointer: 0
Smart pointer: 0
Smart pointer: 4
Smart pointer: 47
Smart pointer: 477
Smart pointer: 4985
Smart pointer: 48349
Copy: 0
Copy: 1
Copy: 16
Copy: 117
Copy: 1187
Copy: 12405
Copy: 123132
Smart is 2.546733 times faster

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Aug 17 2019, 1:39 PM

elexis edited the test plan for this revision. (Show Details)Aug 17 2019, 1:40 PM

elexis edited the summary of this revision. (Show Details)Aug 17 2019, 1:45 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/gui/CGUISprite.h
| 176| »   mutable·CRect·m_CachedSize;
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCClientArea{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/406/display/redirect

Harbormaster completed remote builds in B8894: Diff 9369.Aug 17 2019, 1:48 PM

This revision was not accepted when it landed; it landed in state Needs Review.Aug 17 2019, 1:53 PM

Closed by commit rP22682: Use shared_ptr to avoid copying and deleting SGUIImageEffects, 2.5x faster in a… (authored by elexis). · Explain Why

This revision was automatically updated to reflect the committed changes.

elexis added a commit: rP22682: Use shared_ptr to avoid copying and deleting SGUIImageEffects, 2.5x faster in a….