IRC logs for #openttd on OFTC at 2025-12-19
⏴ go to previous day
00:53:03 *** MinchinWeb[m] has quit IRC (Remote host closed the connection)
00:53:45 *** MinchinWeb[m] has joined #openttd
01:01:29 *** tokai|noir has joined #openttd
01:01:29 *** ChanServ sets mode: +v tokai|noir
01:08:30 *** tokai has quit IRC (Ping timeout: 480 seconds)
02:05:32 *** MinchinWeb[m] has quit IRC (Ping timeout: 480 seconds)
02:05:35 *** MinchinWeb[m] has joined #openttd
02:40:56 *** Zathras_7 has quit IRC (autokilled: This host violated network policy. Mail support@oftc.net if you think this is in error. (2025-12-19 02:40:55))
03:02:12 *** Wormnest has quit IRC (Quit: Leaving)
04:43:23 <DorpsGek> - Update: Translations from eints (by translators)
04:49:03 *** MinchinWeb[m] has quit IRC (Read error: Connection reset by peer)
04:49:21 *** MinchinWeb[m] has joined #openttd
05:24:16 *** Flygon has quit IRC (Read error: Connection reset by peer)
06:19:55 <Rubidium> xarick: can you test the performance of the individual commits of 14937? Maybe that can give an idea where the potential regression lies
07:53:36 *** SigHunter_ has quit IRC (Ping timeout: 480 seconds)
07:59:35 *** SigHunter has joined #openttd
08:50:43 *** SigHunter has joined #openttd
10:01:42 * LordAro submits a merge request review with 27 comments, and a complaint about being made to review an MR with 344 commits and 570 files changed
10:07:27 <Rubidium> that sounds like a good few days of work
10:09:06 <LordAro> luckily i am only actually /required/ to review ~20 files (and 200 of the 570 are just incrementing a version number)
10:09:13 <LordAro> so i scrolled through everything else
10:09:43 <LordAro> but good grief they don't make this easy
10:59:17 <kuhnovic> I had a coworker who always took the opportunity to apply some code style changes and other cleanup, and mix it in with the actual functional changes. And of course this was all in a single commit.
11:01:53 <ahyangyi> They really like `git blame`.
11:02:14 <kuhnovic> It was often 500 lines of cleanup with 30 lines of actually changes. At some people refused to review his work.
11:04:47 *** toktik is now known as Guest33937
11:12:19 *** Guest33937 has quit IRC (Ping timeout: 480 seconds)
11:21:14 <xarick> i hate this much variance
11:21:28 <xarick> should i disable garbage collection again?
11:25:46 <xarick> let me share my Performance Tests GS, I'll do a clean up version of it
11:34:34 <LordAro> sample size of 1 is never ideal
11:34:55 <LordAro> but i'm guessing those numbers are in seconds, so rerunning 10 times will add up...
11:35:16 <ahyangyi> A good Sample 1 is a good reason to start collecting Sample 2 though
11:35:22 <LordAro> i'd probably say it's the last commit though
11:36:00 <ahyangyi> But, is larger values worse or better this time :S
11:49:45 <xarick> make sure you have RAM
11:49:57 <xarick> like... lots, about 16 GB
11:57:27 <xarick> oops line 128 can be deleted
12:04:32 <Borg> I have question about var 0x85 (TTD flags), I see that not all bits are defined.. for example in first range.. there are a lot of gaps.. first bit used is 12..
12:05:00 <Borg> so, I assume that not defined bits are 0 and.. is it safe to grab one of those for some feature? ;)
12:27:03 <Borg> Rubidium: thats ok.. im targeting OpenTTD only..
12:39:51 *** reldred has quit IRC (Quit: User went offline on Discord a while ago)
12:43:14 <jfkuayue> *entering deutschland from padborg -> flensburg*
13:37:16 *** Wormnest has joined #openttd
13:38:29 <peter1138> has_value() & value(), but no need for the temporaries.
13:39:33 <peter1138> Of course, likely being AI-slop derived code it doesn't matter.
13:40:28 *** merni has quit IRC (Quit: User went offline on Discord a while ago)
13:43:30 <xarick> but I've seen both styles already
13:50:58 <xarick> oh, the commit message has a bit of copilotry
13:54:40 <xarick> mostly an invisible issue unless you use mac
13:55:23 <xarick> and you really have to do shennanigans to make it crash
14:01:28 <xarick> making 14772 crash again, just to be sure glx fix works
14:17:40 <xarick> Rito13 i have something
14:31:04 <rito12_51026> Nice it works much better than #13545
15:18:26 <xarick> I'm bored, so I'm gonna run rubidium ValuateTests 5 times and pick best result
15:26:35 <rito12_51026> Shouldn't you like calculate the average and standard deviation?
15:47:58 <kuhnovic> Out comes the black art of statistics
16:11:22 *** MinchinWeb[m] has quit IRC (Ping timeout: 480 seconds)
16:12:42 *** MinchinWeb[m] has joined #openttd
16:14:39 <xarick> im not versed in statistics
16:19:14 <xarick> openttd master g598d8fd65c: 156 157 156 156 156
16:19:14 <xarick> openttd rubidium script-list 1.0 - move to super class: 155 157 156 156 155
16:19:14 <xarick> openttd rubidium script-list 1.1 - constructors const: 153 153 154 154 155
16:19:14 <xarick> openttd rubidium script-list 1.2 - deduplicate by templating: 159 157 157 157 156
16:26:38 *** gelignite has joined #openttd
16:40:15 <_glx_> Valuate should not be really affected by this change
16:45:17 *** MinchinWeb[m] has quit IRC (Ping timeout: 480 seconds)
16:56:23 *** MinchinWeb[m] has joined #openttd
17:12:06 *** MinchinWeb[m] has quit IRC (Read error: Connection reset by peer)
17:12:22 *** MinchinWeb[m] has joined #openttd
17:39:05 <xarick> It has a Begin call per list
17:44:41 <_glx_> Begin is not needed for Valuate
17:45:36 <xarick> ListToListTests (30'000'000)
17:45:36 <xarick> openttd master g598d8fd65c: 227 225 225 226 226
17:45:36 <xarick> openttd rubidium script-list 1.0 - move to super class: 227 228 227 226 226
17:45:36 <xarick> openttd rubidium script-list 1.1 - constructors const: 225 225 226 226 225
17:45:36 <xarick> openttd rubidium script-list 1.2 - deduplicate by templating: 235 236 236 236 235
17:46:25 <xarick> also it constructs about 4473 * 2 lists, that calls the sorter i guess
17:48:20 <_glx_> the templating mostly affects Begin() and FindNext()
17:51:38 <xarick> not sure if .Sort also calls the constructor
17:52:44 <_glx_> the 2 Sort calls in ZeroLists are useless
17:53:26 <xarick> biased towards my other work :)
17:54:02 <_glx_> and basically the double foreach 🙂
17:54:59 <_glx_> Valuate don't care about the Sorter, it's always by ascending items
17:55:44 <xarick> "my other work" doesn't set values in this->values if the sorter is by item
17:55:57 <xarick> that's how it slashes time in half
17:56:16 <xarick> but yeah, master still doesn't do that
17:59:20 <_glx_> anyway the timings for ValuateTests don't really show a difference, 157 or 156 is error margin
17:59:21 *** MinchinWeb[m] has quit IRC (Read error: Connection reset by peer)
17:59:46 <xarick> what I see: 1.1 is the best version
18:00:07 <_glx_> but master and the full PR are equivalent
18:00:17 <_glx_> and that's the important point
18:00:29 <_glx_> for ListToListTests it's different
18:00:49 <_glx_> 10s diff seems consistant
18:01:27 <_glx_> while master vs 1.1 is error margin
18:01:44 <xarick> i have yet to start ther other test, it's 460~ish seconds per run
18:01:44 *** MinchinWeb[m] has joined #openttd
18:02:43 <xarick> that's gonna take me... 153 minutes without touching the computer, not sure I can hold
18:04:18 <xarick> i presume it's the most relevant test too
18:08:52 <_glx_> 99-102 is probably affected
18:09:58 <_glx_> 109-113 too (looks like a list.RemoveArray() call)
18:12:40 <_glx_> so yeah FindNext() is most likely the issue
18:12:57 <peter1138> So, RC3 this weekend?
18:13:10 <xarick> the reason I have a test just for Begin is due to "my other work"
18:13:43 <xarick> Begin forces this-list->values to be built
18:13:43 <_glx_> Begin() is not called enough to be visible in the result
18:14:20 <_glx_> even if the templated version might be a little slower
18:14:34 <_glx_> the biggest impact is FindNext()
18:16:18 <xarick> this one shouldn't result in the 10 seconds extra
18:16:26 <xarick> but it does... for some obscure reason
18:17:54 <_glx_> ahah 75-77 can be just `main[test](aux)`
18:18:39 <_glx_> oh maybe not, lists are weird
18:18:47 <_jgr_> It may be worth considering the use of a profiler, that would give a clearer indication rather than just guessing
18:24:30 <_glx_> KeepList and RemoveList potentially call RemoveItem, which then calls sorter's Remove potentially calling FindNext
18:39:49 <xarick> the sorter is not initialized
18:40:11 <xarick> this was the kind of optimizations that JGR done
18:41:11 <xarick> line 504: if (this->initialized) this->sorter->Remove(item);
18:42:42 <xarick> line 561: if (this->initialized) this->sorter->Remove(item);
18:45:38 <xarick> maybe in .Clear : this->initialized = false;
18:49:19 <Rubidium> my quick and dirty performance test does: master 27.55, commit#1 27.61, commit#2 27.53, commit#3 27.49 | O2, gcc, NDEBUG, array with 500k elements (increasing item, random value), 250 times Sort call with each of the sorters/directions and iterating from Begin to IsEnd
18:50:33 <Rubidium> but with all the efficiency cores, cpu powering down and so on it's not trivial to get a consistent benchmark on a laptop ;(
19:01:02 <xarick> risky proposition, but regression passed still
19:01:53 *** WormnestAndroid has quit IRC (Read error: Connection reset by peer)
19:01:54 *** WormnestAndroid has joined #openttd
19:02:09 *** WormnestAndroid has quit IRC (Read error: Connection reset by peer)
19:02:10 *** WormnestAndroid has joined #openttd
19:02:29 *** WormnestAndroid has quit IRC (Read error: Connection reset by peer)
19:02:30 *** WormnestAndroid has joined #openttd
19:21:22 <rito12_51026> Should sentence in a comment end with a period?
19:22:39 <peter1138> Only when you are writing new comment.
19:23:58 <rito12_51026> So sentences in all comments created in a PR should end with a period
19:28:32 *** brickblock19280 has joined #openttd
19:28:32 <brickblock19280> this isn't ideal
19:40:35 <brickblock19280> brickblock19280: imo the new engine string is better suited to as it is generic and already used to introduce the railtype of the train
19:41:51 <brickblock19280> hmm maybe not
19:54:28 <andythenorth> is it possible that the issue there is poor railtype newgrf choice?
20:05:59 <brickblock19280> peter1138: the yellow at the end of the name of the first railtype is not reset back to gold
20:06:39 <brickblock19280> it's there in order to change the colour of the speed limit which the game adds in the drop down
20:07:33 <brickblock19280> It could be considered a newgrf fault but given the game forces strings into the right colours in other cases I don't see it as such
20:08:47 <brickblock19280> It would also be nice if there was some way to show a more generic and shorter string such as "Standard Gauge 25kv AC"
20:10:47 <peter1138> If only there was a way to shorter string instead of a long string...
20:11:58 <brickblock19280> there was never a need for a short string in the old context
20:12:58 <peter1138> Oh, there's a PR to fix that that I never merged :o
20:14:40 <brickblock19280> SETS but I imagine JP+ has the same issue
20:15:22 <brickblock19280> Id still like not to show overgrown in that context since it's equivilent to the others in terms of compatibility
20:15:43 <peter1138> That's not how railtypes work.
20:16:39 <brickblock19280> true but we can already specify 6 or so strings so one more wouldn't hurt
20:19:39 <peter1138> Dunno what vehicle set you're using though.
20:20:05 <rito12_51026> Doesn't it remove the ability to recolour the speed part of the dropdown that BrickBlock is using?
20:20:32 <brickblock19280> but this is honestly better
20:20:42 <peter1138> Yes, but that was never intended to be allowed anyway.
20:20:43 <brickblock19280> I just don't think that should be black
20:24:15 <_glx_> you can still change colours inside substring, but it should not affect the parent string
20:25:43 <brickblock19280> yeah but the speed is in the parent string
20:27:39 <peter1138> It's possible to make it yellow. Submit a PR to change the string in english.txt. And then every railtype with speedlimits gets a yellow speedlimit instead of having to rely on a bug.
20:29:54 <peter1138> Anyway, I labelled #14006 for backport--although it's labelled as "Change" it's probably actually a "Fix". But I can unlabel it.
20:31:45 <peter1138> brickblock19280, what vehicle set were you using?
20:33:14 <brickblock19280> although I did just remove that code
20:33:19 <brickblock19280> so it's not in there
20:34:21 <brickblock19280> i'll send you a new one once it compiles
20:35:42 <brickblock19280> this should have it
20:39:18 <peter1138> As ever, Discord is a crap place for distribution of assets.
20:39:31 <peter1138> (I cannot download those.)
20:41:13 <kuhnovic> It's only crap if you're on IRC 😉
20:41:20 <peter1138> It's crap because it's Discord.
20:42:52 <rito12_51026> It's better than messenger
20:47:14 <Borg> peter1138: yeah, in good old times people IRCed from shell accounts.. and automagicaly.. every user had HTTP server at their disposal with ~/<nick>/ to share whatever stuff they needed
20:47:30 <Borg> but.. well.. progress...
21:16:14 <xarick> static const size_t SCRIPT_LIST_BYTES_PER_ITEM = 64;
21:17:47 <xarick> I would go with 8+8+8+8
21:18:12 <xarick> two SQInteger in map and two other in set
21:19:02 <xarick> but there's extras for the rest though
21:20:21 <Rubidium> how many bytes does your tree use per element?
21:21:13 <xarick> 1500 per leaf, not really a per item thing
21:22:42 <xarick> each leaf can store up to 63 without splitting, splits on 64
21:23:39 <Rubidium> just naively 16 bytes for the malloc header, 16 bytes for the two children and 16 bytes for the two SQIntegers, round up to next 2**n boundary and you're already at 64 bytes
21:27:33 <_glx_> couldn't we overload map/set Allocator to account it inside _squirrel_allocator ?
21:30:48 <xarick> let's test 14939, wanna see the memory usage vs what task manager tells
21:32:40 <xarick> or even visual studio graph
21:35:16 *** Borg has quit IRC (Quit: nite)
21:36:45 <xarick> okay it actually double
21:39:53 <xarick> peaked at 11.4 GB, but it stalled, couldn't see what the ingame perf reported :|
21:42:58 <peter1138> _glx_, I think you can set the allocator for items, but not for the internal data structures.
21:46:14 <_jgr_> The std::set and std::map are going to have 32 bytes per node overhead even before you add malloc overheads, etc
21:47:03 <xarick> that's 18 million + 18 million
21:47:56 <xarick> now it's gonna do a KeepList step which stalls
21:48:56 <xarick> oops, actually it's cloning before KeepList
21:55:01 <xarick> surprised KeepList didn't reduce the size
21:55:23 <xarick> memory usage should have decreased, :|
22:02:04 <xarick> should we (i mean you) actually make am in-house Btree for ScriptList? memory efficiency
22:06:52 <xarick> kinda cheating because only list->items has stuff
22:07:00 <xarick> let me initialize values
22:09:11 <peter1138> Well, have you considered simply NOT adding 18 million items to a list?
22:09:56 <xarick> 583 MB vs 2.5 GB for the same 18m
22:12:04 <xarick> here's with both this->items and this->values
22:12:25 <xarick> still half of std::map std::set
22:14:55 <xarick> oh i like what glx is doing. It's smoothifying all these stalls
22:15:24 <xarick> might end making some AIs snooze
22:16:22 <_glx_> yup that's the idea, spread the heavy processing caused by scripts doing silly things
22:18:17 <xarick> now picking the best trains from Iron Horse is just gonna take centuries
22:19:47 <xarick> don't forget AddRectangle/RemoveRectangle
22:20:58 <_glx_> tile iterators are not easily resumable
22:22:14 <_jgr_> The practical upshot of accounting for script list memory is that you can just kill the script due to exceeding its memory budget if it tries to AddRectangle the entire map 😛
22:23:19 <_jgr_> (AddRectangle then Valuate is a godawful way to scan tiles, but that ship has sailed already unfortunately)
22:24:46 <peter1138> Version that 1) uses the real allocation size, so completely ignores internal allocation :( but 2) doesn't require us to add/remove manually.
22:26:27 *** Wolf01 has quit IRC (Quit: Once again the world is quick to bury me.)
22:27:05 <peter1138> I guess if you know what the average internal allocation size is that could also be included.
22:31:19 <peter1138> _glx_, for AddRectangle/RemoveRectangle, could it be taken out of the API and make into a squirrel-side function? Kinda like a compat function.
22:31:45 <peter1138> It would of course be slower, but also automatically suspendable.
22:32:25 <_jgr_> The argument to allocate/deallocate is the allocation size, it shouldn't be multiplied by sizeof(T), and it would include the rest of the node struct already
22:32:52 <peter1138> _jgr_, you'd think so, but without it the value is "1" every time.
22:33:04 <peter1138> And I'm doubtful it's allocating a byte at a time.
22:35:35 <xarick> i experimented with emplace_hint on AddRectangle. Makes the whole thing much faster
22:36:18 <peter1138> > Allocates `n * sizeof(T)` bytes of uninitialized storage
22:36:23 <_glx_> `n` is the number of allocated items
22:36:50 <_jgr_> Yes, you're right, that seems a bit useless, though?
22:39:40 <peter1138> If there were other data structures that we wanted to account for as well, there would be more point.
22:39:55 <peter1138> But to account for the internal structure of a map, we basically need to reimplement the map.
22:44:05 <_glx_> it's always possible to double the accounted size
22:47:34 <_jgr_> The actual call to malloc will have the correct size in it
22:51:30 <_jgr_> I expect that `T` in your ScriptStdAllocator instance is not actually the one you passed in to std::set/std::map because it's going through `std::allocator_traits<...>::rebind_alloc`
22:51:50 <_jgr_> In which case, it is fine as is (?)
22:52:06 <peter1138> Some kind of magic then.
22:52:17 <peter1138> It was 48 pints, which seems wrong for the actual type I passed.
22:52:22 <peter1138> So maybe it is real after all.
22:52:35 <_jgr_> That sounds right for a red-black tree node
22:57:29 <peter1138> I don't think I have a script lying around that abuses ScriptList like xarick does :)
23:07:58 <xarick> will you consider a custom btree for scriptlist? I don't say mine
23:09:13 <xarick> benefits are speed and memory usage
23:12:18 <xarick> i wonder what errors will CI come up with
23:17:42 <peter1138> Slop-coding, just fuck off with that.
23:18:43 <xarick> how do you tell right away?
23:19:47 <xarick> anyway the only important file
23:22:55 <xarick> it was a very nice experiment
23:24:17 <xarick> it surprised me in a positive way, that the ai could guide me all the way to a fully working piece of code
23:24:51 <xarick> in this case a bplustree. I went in blindly, with no idea how a btree work
23:26:58 <xarick> I still don't trust it to be bug free...
23:27:13 <xarick> but right now, it produces correct results
23:33:44 <xarick> so... maybe someone more experienced could make a OpenTTD flavour of btree for ScriptList :)
23:36:07 <_zephyris> Soo, how am I being dumb `make: *** No rule to make target 'sprites/png/gui/badger_menu_icon.png', needed by 'ogfxe_extra.grf'. Stop.`
23:37:37 <xarick> going to bed. Gonna leave this running RemoveItemTests for rubidium, 10 iterations
continue to next day ⏵
