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

Introduce a faster linked list implementation #32234

Merged
merged 10 commits into from
Jul 8, 2019

Conversation

ifreund
Copy link
Contributor

@ifreund ifreund commented Jul 7, 2019

Summary

SUMMARY: Infrastructure "Introduce a faster linked list implementation"

Purpose of change

The new 3d shadowcasting algorithm in #32012 uses an std::list which is not ideal in performance critical code as std::list is relatively slow.

Describe the solution

The plflib list implementation is much faster than std::list, so I've imported that just like colony.

Quoting from https://www.plflib.org/list.htm:

plf::list is a drop-in higher-performance replacement for std::list, with (on average) *:

  • 293% faster insertion
  • 57% faster erasure
  • 17% faster iteration
  • 77% faster sorting
  • 70% faster reversal
  • 91% faster remove/remove_if
  • 63% faster unique
  • 811% faster clear (1147900% for trivially-destructible types)
  • 1248% faster destruction (6350% for trivially-destructible types)
  • 20-24% faster performance overall in ordered use-case benchmarking(insertion, erasure and iteration on the fly and over time)

Like std::list, plf::list maintains stable pointer/iterator links to list elements regardless of erasure, insertion, merging, splicing, reversing and sorting. It's functionality replicates std::list with two minor exceptions: the removal of incomplete list splices (splicing whole lists is still available) and inclusion of several useful functions: reserve(), shrink_to_fit(), free_unused_memory(), and approximate_memory_use().

Describe alternatives you've considered

Not making things faster

Additional context

This PR merely imports the the list implementation, adjusts it to meet our needs, and adds tests.

In the next PR I plan to replace all uses of std::list in the codebase with cata::list as it is a drop in replacement with no downsides aside from the removal of incomplete list splicing.

@ifreund ifreund added Code: Performance Performance boosting code (CPU, memory, etc.) Code: Tests Measurement, self-control, statistics, balancing. [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style labels Jul 7, 2019
@kevingranade
Copy link
Member

My only concern with swapping this out for std::list everywhere is from a cursory glance it looks like the first insertion into an empty list is going to be disproportionately expensive since the first block allocation will be large. If it does some kind of size increase strategy this concern is moot.

@ifreund
Copy link
Contributor Author

ifreund commented Jul 8, 2019

As far as I can tell there is at least some kind of size increase strategy as there are minimum and maximum block sizes defined and the first allocation is always of the minimum block size. I haven't looked into this in detail though and can't yet tell you what that minimum size is aside from

#define LIST_BLOCK_MIN static_cast<group_size_type>((sizeof(node) * 8 > (sizeof(*this) + sizeof(group)) * 2) ? 8 : (((sizeof(*this) + sizeof(group)) * 2) / sizeof(node)) + 1)

Will look into it more in-depth this evening

@ifreund
Copy link
Contributor Author

ifreund commented Jul 8, 2019

Alright I did some profiling and here are some numbers for lists of ints:

number of elements size delta
0 112 b
1 432 b + 320 b
12 432 b
13 752 b + 320 b
24 752 b
25 1392 b + 640 b
48 1392 b
49 2672 b + 1280 b

As can be pretty clearly seen, this list implementation uses a growth factor of two, same as colony. I think this makes it fine for general use, but it's your call really.

@kevingranade
Copy link
Member

No that's totally fine, I was guessing there was a growth factor, but wanted to verify.
I did some reading too, it looks like it actually amortizes the initial growth away over time by preferring to delete the inital generations if they ever empty out, which is pretty cool.

@kevingranade kevingranade merged commit f8ee22c into CleverRaven:master Jul 8, 2019
@ifreund ifreund deleted the better-list branch July 9, 2019 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style Code: Performance Performance boosting code (CPU, memory, etc.) Code: Tests Measurement, self-control, statistics, balancing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants