Skip to content

perf: a bunch of optimization - #2417

Open
SimplisticMind wants to merge 2 commits into
ModOrganizer2:masterfrom
SimplisticMind:performance
Open

perf: a bunch of optimization#2417
SimplisticMind wants to merge 2 commits into
ModOrganizer2:masterfrom
SimplisticMind:performance

Conversation

@SimplisticMind

Copy link
Copy Markdown

This is a large one so if you want to just cherrypick - feel free to do so; with that out of the way, a few less obvious ones:
envfs.h - upwards of ~20%+ faster refreshing
moapplication.cpp - Qt elements render up to twice as fast
settings.cpp - most impactful settings got hoisted out, other ones aren't hot enough to really necessitate such hoisting too
fileregister.cpp - sortOrigins() - 4x faster compared to locking each file seperately

@Holt59

Holt59 commented Jun 27, 2026

Copy link
Copy Markdown
Member

Thanks for the PR, it would be easier and more likely to be accepted if you split it in smaller chunk because as it stands, it's complicated to review (and cherry-picking would take a lot of time).

schlosserleo added a commit to mo2-modern/modorganizer that referenced this pull request Aug 15, 2026
ModOrganizer2#2417 by SimplisticMind, +468/-120 across 19 files
in the file-tree and plugin-list core: organizercore, pluginlist,
directoryentry, fileregister, fileentry, directoryrefresher.

Merged rather than cherry-picked, deliberately. The PR's own commits keep
their SHAs in our history, so provenance stays legible and a later merge of
master can recognise them if upstream lands them as-is. If upstream squashes
instead, that benefit is lost and the duplicate will have to be resolved
then -- worth knowing rather than being surprised by.

⚠️ It is UNMERGED and UNREVIEWED upstream: opened 2026-06-24, zero reviews,
REVIEW_REQUIRED. Upstream CI is green, but upstream does not build with /WX
while every C++ repo here does, so their green says less about this tree
than it looks. If upstream revises it under review, our copy diverges and
this merge is the thing that has to be redone.

One conflict, in fileregister.h, and the resolution is the union of both
sides: their std::mutex -> std::shared_mutex change plus the second
m_OriginsSortMutex, keeping our `FileMap m_Files{}` from the member-init
sweep. FileMap is a std::deque, so `{}` is not the allocator-aware hazard
TRAPS warns about, and modern has shipped it that way already.

Checked that the auto-merge did not quietly drop our work in the twelve
files both sides touched: the unchecked-file.open fix in pluginlist.cpp
survives, so does organizercore.cpp's preview-failure logging, the
metaType() deprecation fix in settings.cpp, and all thirteen member-init
sites in the overlapping headers.
schlosserleo added a commit to mo2-modern/modorganizer that referenced this pull request Aug 15, 2026
…dropped downloads)

ModOrganizer2#2406 by TwistedModding, +135/-53 across 6 files
in the download manager and the single-instance IPC: downloadlist,
downloadmanager, multiprocess.

⚠️ Weaker upstream evidence than ModOrganizer2#2417. It is unmerged and unreviewed
(opened 2026-06-12, REVIEW_REQUIRED), and where ModOrganizer2#2417 at least had upstream
build and lint checks pass, this one has only pre-commit.ci -- upstream CI
has never compiled it. Our /W4 /WX build is now the only compiler that has.

version.rc is deliberately resolved to OURS, discarding their third commit
entirely. It bumps the version to 2.5.3beta12 and sets FILEFLAGS to
VS_FF_PRERELEASE, neither of which has anything to do with fixing downloads
-- it is the author's own release prep riding along in a bugfix PR. Taking
it would have renamed our build mid-merge and flipped createVersionInfo()
onto the string-parsing branch, which is exactly the path ADR-028 chose the
numeric side of. We stay at 2.5.2.1 with FILEFLAGS 0.

That upstream is cutting 2.5.3 betas is worth noting separately: when they
ship it, ADR-028 says we rebase onto 2.5.3 and become 2.5.3.1.

modern also touches downloadmanager.cpp/.h, so the auto-merge was checked
rather than trusted: all nine member-init sites survive, as does the
TU-local `using namespace boost::accumulators` that keeps its extractors out
of every includer.
schlosserleo added a commit to mo2-modern/modorganizer that referenced this pull request Aug 15, 2026
SimplisticMind (ModOrganizer2#2417, perf) and TwistedModding (ModOrganizer2#2406, download manager
fixes) both wrote code this fork now ships, so both go in the About
dialog's "MO2 Developers & Contributors" list.

Two judgement calls worth stating rather than burying.

SimplisticMind was already in the dialog, under "Other Supporters &
Contributors". That entry is upstream's and is left exactly as it is --
editing it would be editorialising on upstream's own structure for no gain
-- so the name now appears twice, in two lists that mean different things.
That is mildly odd to look at and more accurate than either alternative.

TwistedModding is the GitHub handle; the git author on those commits reads
"Teddy Lybold". The handle is used because it matches how the PR is
attributed and how the surrounding entries are written, several of which are
handles. Easy to change if they would rather be credited by name.

Neither replaces the credit that already exists in git: both PRs were merged
rather than cherry-picked, so their commits keep their original authorship
and SHAs.
schlosserleo added a commit to mo2-modern/modorganizer that referenced this pull request Aug 15, 2026
Second fork release. Carries upstream PRs ModOrganizer2#2417 (perf) and ModOrganizer2#2406 (download
manager and single-instance IPC fixes), both merged and both confirmed
working by the owner: the mod list under ModOrganizer2#2417's file-tree changes, and the
download and second-instance paths under ModOrganizer2#2406's.

Fourth segment only, per ADR-028. Upstream still owns 2.5.3, and is
currently cutting betas of it -- when they ship, this rebases onto 2.5.3 and
becomes 2.5.3.1.

Fork identity; do not include in any PR branch cut from master.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants