Skip to content

Commit

Permalink
[PORT] sorting code improvements+optimizations (#4797)
Browse files Browse the repository at this point in the history
* Micros timSort slightly (#74889)

## About The Pull Request

Datum var reads are expensive, and sorting code does a lot of them.
let's work on that together, as a group.
There's maybe 250ms of sorting cost sitting in mostly global variable
work. I'd like to start chopping at that.

My tracy profiles aren't the most helpful, estimating this to save about
14% of timSort over the course of init, tho that's slightly noisy and
not the most reliable

## Why It's Good For The Game
speed

* Code cleanup: Sorting (#83017)

1. Removes code duplication
2. Fully documents `sortTim()`
3. Makes a define with default sortTim behavior, straight and to the
point for 95% of cases
4. Migrates other sorts into the same file
5. Removes some redundancy where they're reassigning a variable using an
in place sorter

For the record, we only use timSort
More documentation, easier to read, uses `length` over `len`, etc
Should be no gameplay effect at all

---------

Co-authored-by: LemonInTheDark <[email protected]>
Co-authored-by: Jeremiah <[email protected]>
  • Loading branch information
3 people authored Jan 12, 2025
1 parent 7620fa3 commit f978433
Show file tree
Hide file tree
Showing 16 changed files with 116 additions and 75 deletions.
2 changes: 1 addition & 1 deletion code/__HELPERS/hallucinations.dm
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ GLOBAL_LIST_INIT(random_hallucination_weighted_list, generate_hallucination_weig
last_type_weight = this_weight

// Sort by weight descending, where weight is the values (not the keys). We assoc_to_keys later to get JUST the text
all_weights = sortTim(all_weights, GLOBAL_PROC_REF(cmp_numeric_dsc), associative = TRUE)
sortTim(all_weights, GLOBAL_PROC_REF(cmp_numeric_dsc), associative = TRUE)

var/page_style = "<style>table, th, td {border: 1px solid black;border-collapse: collapse;}</style>"
var/page_contents = "[page_style]<table style=\"width:100%\">[header][jointext(assoc_to_keys(all_weights), "")]</table>"
Expand Down
19 changes: 0 additions & 19 deletions code/__HELPERS/sorts/InsertSort.dm

This file was deleted.

19 changes: 0 additions & 19 deletions code/__HELPERS/sorts/MergeSort.dm

This file was deleted.

20 changes: 0 additions & 20 deletions code/__HELPERS/sorts/TimSort.dm

This file was deleted.

95 changes: 95 additions & 0 deletions code/__HELPERS/sorts/helpers.dm
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/// Sorts the list in place with timSort, default settings.
#define SORT_TIM(to_sort, associative) if(length(to_sort) >= 2) { \
var/datum/sort_instance/sorter = GLOB.sortInstance; \
if (isnull(sorter)) { \
sorter = new; \
} \
sorter.L = to_sort; \
sorter.cmp = GLOBAL_PROC_REF(cmp_numeric_asc); \
sorter.associative = associative; \
sorter.timSort(1, 0); \
}


/// Helper for the sorting procs. Prevents some code duplication. Creates /datum/sort_instance/sorter
#define CREATE_SORT_INSTANCE(to_sort, cmp, associative, fromIndex, toIndex) \
if(length(to_sort) < 2) { \
return to_sort; \
} \
fromIndex = fromIndex % length(to_sort); \
toIndex = toIndex % (length(to_sort) + 1); \
if (fromIndex <= 0) { \
fromIndex += length(to_sort); \
} \
if (toIndex <= 0) { \
toIndex += length(to_sort) + 1; \
} \
var/datum/sort_instance/sorter = GLOB.sortInstance; \
if (isnull(sorter)) { \
sorter = new; \
} \
sorter.L = to_sort; \
sorter.cmp = cmp; \
sorter.associative = associative;


/**
* ## Tim Sort
* Hybrid sorting algorithm derived from merge sort and insertion sort.
*
* **Sorts in place**.
* You might not need to get the return value.
*
* @see
* https://en.wikipedia.org/wiki/Timsort
*
* @param {list} to_sort - The list to sort.
*
* @param {proc} cmp - The comparison proc to use. Default: Numeric ascending.
*
* @param {boolean} associative - Whether the list is associative. Default: FALSE.
*
* @param {int} fromIndex - The index to start sorting from. Default: 1.
*
* @param {int} toIndex - The index to stop sorting at. Default: 0.
*/
/proc/sortTim(list/to_sort, cmp = GLOBAL_PROC_REF(cmp_numeric_asc), associative = FALSE, fromIndex = 1, toIndex = 0) as /list
CREATE_SORT_INSTANCE(to_sort, cmp, associative, fromIndex, toIndex)

sorter.timSort(fromIndex, toIndex)

return to_sort


/**
* ## Merge Sort
* Divide and conquer sorting algorithm.
*
* @see
* - https://en.wikipedia.org/wiki/Merge_sort
*/
/proc/sortMerge(list/to_sort, cmp = GLOBAL_PROC_REF(cmp_numeric_asc), associative = FALSE, fromIndex = 1, toIndex = 0) as /list
CREATE_SORT_INSTANCE(to_sort, cmp, associative, fromIndex, toIndex)

sorter.mergeSort(fromIndex, toIndex)

return to_sort


/**
* ## Insertion Sort
* Simple sorting algorithm that builds the final sorted list one item at a time.
*
* @see
* - https://en.wikipedia.org/wiki/Insertion_sort
*/
/proc/sortInsert(list/to_sort, cmp = GLOBAL_PROC_REF(cmp_numeric_asc), associative = FALSE, fromIndex = 1, toIndex = 0) as /list
CREATE_SORT_INSTANCE(to_sort, cmp, associative, fromIndex, toIndex)

sorter.binarySort(fromIndex, toIndex)

return to_sort


#undef CREATE_SORT_INSTANCE
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ GLOBAL_DATUM_INIT(sortInstance, /datum/sort_instance, new())
if(start <= lo)
start = lo + 1

for(,start < hi, ++start)
var/list/L = src.L
for(start in start to hi - 1)
var/pivot = fetchElement(L,start)

//set left and right to the index where pivot belongs
Expand Down Expand Up @@ -140,6 +141,7 @@ GLOBAL_DATUM_INIT(sortInstance, /datum/sort_instance, new())
if(runHi >= hi)
return 1

var/list/L = src.L
var/last = fetchElement(L,lo)
var/current = fetchElement(L,runHi++)

Expand Down Expand Up @@ -221,7 +223,6 @@ GLOBAL_DATUM_INIT(sortInstance, /datum/sort_instance, new())
runLens.Cut(i+1, i+2)
runBases.Cut(i+1, i+2)


//Find where the first element of run2 goes in run1.
//Prior elements in run1 can be ignored (because they're already in place)
var/k = gallopRight(fetchElement(L,base2), base1, len1, 0)
Expand Down Expand Up @@ -259,6 +260,7 @@ GLOBAL_DATUM_INIT(sortInstance, /datum/sort_instance, new())
/datum/sort_instance/proc/gallopLeft(key, base, len, hint)
//ASSERT(len > 0 && hint >= 0 && hint < len)

var/list/L = src.L
var/lastOffset = 0
var/offset = 1
if(call(cmp)(key, fetchElement(L,base+hint)) > 0)
Expand Down Expand Up @@ -318,6 +320,7 @@ GLOBAL_DATUM_INIT(sortInstance, /datum/sort_instance, new())
/datum/sort_instance/proc/gallopRight(key, base, len, hint)
//ASSERT(len > 0 && hint >= 0 && hint < len)

var/list/L = src.L
var/offset = 1
var/lastOffset = 0
if(call(cmp)(key, fetchElement(L,base+hint)) < 0) //key <= L[base+hint]
Expand Down Expand Up @@ -366,6 +369,7 @@ GLOBAL_DATUM_INIT(sortInstance, /datum/sort_instance, new())
/datum/sort_instance/proc/mergeLo(base1, len1, base2, len2)
//ASSERT(len1 > 0 && len2 > 0 && base1 + len1 == base2)

var/list/L = src.L
var/cursor1 = base1
var/cursor2 = base2

Expand Down Expand Up @@ -468,6 +472,7 @@ GLOBAL_DATUM_INIT(sortInstance, /datum/sort_instance, new())
/datum/sort_instance/proc/mergeHi(base1, len1, base2, len2)
//ASSERT(len1 > 0 && len2 > 0 && base1 + len1 == base2)

var/list/L = src.L
var/cursor1 = base1 + len1 - 1 //start at end of sublists
var/cursor2 = base2 + len2 - 1

Expand Down Expand Up @@ -610,6 +615,7 @@ GLOBAL_DATUM_INIT(sortInstance, /datum/sort_instance, new())
return L

/datum/sort_instance/proc/mergeAt2(i)
var/list/L = src.L
var/cursor1 = runBases[i]
var/cursor2 = runBases[i+1]

Expand Down
4 changes: 2 additions & 2 deletions code/controllers/subsystem/dcs.dm
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ PROCESSING_SUBSYSTEM_DEF(dcs)
else
fullid += REF(key)

if(length(named_arguments))
named_arguments = sortTim(named_arguments, GLOBAL_PROC_REF(cmp_text_asc))
if(named_arguments)
sortTim(named_arguments, GLOBAL_PROC_REF(cmp_text_asc))
fullid += named_arguments

return list2params(fullid)
Expand Down
2 changes: 1 addition & 1 deletion code/datums/datum.dm
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@
ASSERT(isatom(src) || isimage(src))
var/atom/atom_cast = src // filters only work with images or atoms.
atom_cast.filters = null
filter_data = sortTim(filter_data, GLOBAL_PROC_REF(cmp_filter_data_priority), TRUE)
sortTim(filter_data, GLOBAL_PROC_REF(cmp_filter_data_priority), TRUE)
for(var/filter_raw in filter_data)
var/list/data = filter_data[filter_raw]
var/list/arguments = data.Copy()
Expand Down
2 changes: 1 addition & 1 deletion code/modules/admin/verbs/debug.dm
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,7 @@
var/list/sorted = list()
for (var/source in per_source)
sorted += list(list("source" = source, "count" = per_source[source]))
sorted = sortTim(sorted, GLOBAL_PROC_REF(cmp_timer_data))
sortTim(sorted, GLOBAL_PROC_REF(cmp_timer_data))

// Now that everything is sorted, compile them into an HTML output
var/output = "<table border='1'>"
Expand Down
2 changes: 1 addition & 1 deletion code/modules/antagonists/malf_ai/malf_ai_module_picker.dm
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
filtered_modules[AM.category][AM] = AM

for(var/category in filtered_modules)
filtered_modules[category] = sortTim(filtered_modules[category], GLOBAL_PROC_REF(cmp_malfmodules_priority))
sortTim(filtered_modules[category], GLOBAL_PROC_REF(cmp_malfmodules_priority))

return filtered_modules

Expand Down
2 changes: 1 addition & 1 deletion code/modules/antagonists/pirate/pirate.dm
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
//Lists notable loot.
if(!cargo_hold || !cargo_hold.total_report)
return "Nothing"
cargo_hold.total_report.total_value = sortTim(cargo_hold.total_report.total_value, cmp = GLOBAL_PROC_REF(cmp_numeric_dsc), associative = TRUE)
sortTim(cargo_hold.total_report.total_value, cmp = GLOBAL_PROC_REF(cmp_numeric_dsc), associative = TRUE)
var/count = 0
var/list/loot_texts = list()
for(var/datum/export/E in cargo_hold.total_report.total_value)
Expand Down
2 changes: 1 addition & 1 deletion code/modules/asset_cache/assets/uplink.dm
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
var/list/items = list()
for(var/datum/uplink_category/category as anything in subtypesof(/datum/uplink_category))
categories += category
categories = sortTim(categories, GLOBAL_PROC_REF(cmp_uplink_category_desc))
sortTim(categories, GLOBAL_PROC_REF(cmp_uplink_category_desc))

var/list/new_categories = list()
for(var/datum/uplink_category/category as anything in categories)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
/datum/computer_file/program/emojipedia/New()
. = ..()
// Sort the emoji list so it's easier to find things and we don't have to keep sorting on ui_data since the number of emojis can not change in-game.
emoji_list = sortTim(emoji_list, /proc/cmp_text_asc)
sortTim(emoji_list, /proc/cmp_text_asc)

/datum/computer_file/program/emojipedia/ui_static_data(mob_user)
var/list/data = list()
Expand Down
2 changes: 1 addition & 1 deletion code/modules/research/stock_parts.dm
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ If you create T5+ please take a pass at mech_fabricator.dm. The parts being good
continue
part_list += component_part
//Sort the parts. This ensures that higher tier items are applied first.
part_list = sortTim(part_list, GLOBAL_PROC_REF(cmp_rped_sort))
sortTim(part_list, GLOBAL_PROC_REF(cmp_rped_sort))
return part_list

/proc/cmp_rped_sort(obj/item/first_item, obj/item/second_item)
Expand Down
2 changes: 1 addition & 1 deletion code/modules/unit_tests/unit_test.dm
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ GLOBAL_VAR_INIT(focused_tests, focused_tests())
if(length(focused_tests))
tests_to_run = focused_tests

tests_to_run = sortTim(tests_to_run, GLOBAL_PROC_REF(cmp_unit_test_priority))
sortTim(tests_to_run, GLOBAL_PROC_REF(cmp_unit_test_priority))

var/list/test_results = list()

Expand Down
6 changes: 2 additions & 4 deletions tgstation.dme
Original file line number Diff line number Diff line change
Expand Up @@ -593,10 +593,8 @@
#include "code\__HELPERS\paths\jps.dm"
#include "code\__HELPERS\paths\path.dm"
#include "code\__HELPERS\paths\sssp.dm"
#include "code\__HELPERS\sorts\__main.dm"
#include "code\__HELPERS\sorts\InsertSort.dm"
#include "code\__HELPERS\sorts\MergeSort.dm"
#include "code\__HELPERS\sorts\TimSort.dm"
#include "code\__HELPERS\sorts\helpers.dm"
#include "code\__HELPERS\sorts\sort_instance.dm"
#include "code\__HELPERS\~monkestation-helpers\announcements.dm"
#include "code\__HELPERS\~monkestation-helpers\antags.dm"
#include "code\__HELPERS\~monkestation-helpers\atoms.dm"
Expand Down

0 comments on commit f978433

Please sign in to comment.