Skip to content

Optimize for size - #185

Open
csware wants to merge 2 commits into
ProgerXP:masterfrom
csware:optimize-for-size
Open

Optimize for size#185
csware wants to merge 2 commits into
ProgerXP:masterfrom
csware:optimize-for-size

Conversation

@csware

Copy link
Copy Markdown
Contributor

This improves issue #183. However, I suppose you/we want to discuss comit 30ddf5c as this introduces a new dependency to the VS2017 C++ Runtime Libraries (which are not present on all Windows systems by default, IIRC).

This MR is supposed to be on top of #184.

@ProgerXP

Copy link
Copy Markdown
Owner

Personally I would choose speed over size. I believe most other users too. I don't think optimizing for speed over size alone is what is making our builds so much larger (#183).

Default configuration should be optimizing for speed.

@ProgerXP

Copy link
Copy Markdown
Owner

Is it normal that GitHub shows diff in this PR with already merged master as if it wasn't merged? Supposedly it should only show new changes in this PR compared to new HEAD.

This saves around 300 KiB w/o boost.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
This saves another 200 KiB, however, this now requires the MSVC++ to be present.
Notepad2 was also linked against this runtime library (requires the old MSVCP60.DLL).
Signed-off-by: Sven Strickroth <email@cs-ware.de>
@cshnik

Copy link
Copy Markdown
Collaborator
  1. Optimization for size allow to decrease binary size for ~300KiB. You've already decided not to use this option by default.
  2. Switching to "Multi-threaded DLL" runtime library (/MD option) allow to decrease binary size for ~200 KiB but caused a dependency from the external runtime DLLs. Unfortunately, this option heavily defeats the portability of the Notepad2e builds and does not allow to start application on clean OSes (even on Windows 10), error prompt will popup:
    image
  3. Summary: total available size decrease is ~500KiB.
Size, KiB/MT/MT + optimization/MD/MD + optimization
Release x862017182117051503
Release x642420214521881915
  1. At this point diff for this PR vs master branch is shown correctly.

Please clarify: what is supposed to be done with the changes from this PR?

@cshnikcshnik assigned ProgerXP and unassigned cshnikAug 28, 2018
@ProgerXP

Copy link
Copy Markdown
Owner

@csware What are we supposed to do with this PR?

@csware

csware commented Feb 21, 2019

Copy link
Copy Markdown
ContributorAuthor

Yesterday I build the vanilla Notepad2 sources (with VS2017) and got a binary of ~800kib, building bf22e43 alrteady resulted in ~1,2mib (x86).

@ProgerXP

Copy link
Copy Markdown
Owner

Yesterday I build the vanilla Notepad2 sources (with VS2017) and got a binary of ~800kib, building bf22e43 alrteady resulted in ~1,2mib (x86).

Could you figure why? Maybe because of the updated Scintilla? Or VC runtime version?

Personally I don't mind the extra 1.5 MiB of size Notepad 2e brings. It's almost completely irrelevant in today's world, it's not like we're talking about 10s or 100s of megabytes gained.

If there's an easy way to shed off some size without sacrificing functionality (Boost, etc.) or speed, then it's fine, but if not, I would not stress too much over it.

@ProgerXP

Copy link
Copy Markdown
Owner

building bf22e43 alrteady resulted in ~1,2mib (x86).

I wonder if it's a coincidence that XhmikosR's mod (https://github.com/XhmikosR/notepad2-mod/) is about the same size, 1.38 MB.

Sign up for freeto 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.

3 participants

@csware@ProgerXP@cshnik