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

VC++ on v12+ builds bloated addon binaries. #29501

Closed
simonbuchan opened this issue Sep 9, 2019 · 10 comments
Closed

VC++ on v12+ builds bloated addon binaries. #29501

simonbuchan opened this issue Sep 9, 2019 · 10 comments
Labels
addons Issues and PRs related to native addons. build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.

Comments

@simonbuchan
Copy link

  • Version: v12.0.0
  • Platform: Windows 10 64-bit
  • Subsystem:

Opening this as a node bug, rather than a node-gyp bug, as it's specific to the target version of node, thus presumably to do with the files node-gyp fetches from nodejs.org/dist/...

I've been maintaining a native module available here for low-level access to the windows notification icon and closely related APIs, and recently found that my .node binary size increased from ~300kB to ~1MB. Since I'm only targeting windows and using N-API, I prebuild the binary and include that in my package so users don't have to install build tools, and I wanted to debug why this increase happened.

After some debugging, I found that this is due to building against node v12.0.0 up:

> yarn rebuild --target v10.16.3
...
>dir notify_icon.node
...
2019-09-09  19:56           377,856 notify_icon.node
...
> yarn rebuild --target v11.13.0
...
>dir notify_icon.node
...
2019-09-09  19:56           377,856 notify_icon.node
...
> yarn rebuild --target v12.0.0
...
>dir notify_icon.node
...
2019-09-09  18:20         1,122,304 notify_icon.node
...
> yarn rebuild --target v12.10.0
...
>dir notify_icon.node
...
2019-09-09  18:20         1,122,304 notify_icon.node
...

Attached is the output of Sizer, which seems to give the most readable output for windows, the output seems to be identical for all before v12 and for all from 12:

Notably, when diffing:

@@ -1,96 +1,202 @@
 Functions by size (kilobytes, min 0.50):
-    1.69: napi_get_cb_info<MenuObject*,int,int>                                            menu-object.obj                          napi_get_cb_info<MenuObject*,int,int>
-    1.69: napi_get_cb_info<MenuObject*,int,menu_item>                                      menu-object.obj                          napi_get_cb_info<MenuObject*,int,menu_item>
-    1.69: napi_get_cb_info<NotifyIconObject*,notify_icon_object_options>                   notify-icon-object.obj                   napi_get_cb_info<NotifyIconObject*,notify_icon_object_options>
-    1.68: napi_get_cb_info<void,icon_size_t,std::optional<unsignedint>,std::optional<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t>>>> icon-object.obj                          enum napi_status __cdecl napi_get_cb_info<void,struct icon_size_t,class std::optional<unsigned int>,class std::optional<class std::basic_string<wchar_t,struct std::char_traits<wchar_t>,class std::allocator<wchar_t> > > >(struct napi_env__ * __ptr64,struct napi_callback_info__ * __ptr64,void * __ptr64,...
-    1.66: napi_register_module_v1                                                          module.obj                               napi_register_module_v1
-    1.66: napi_get_cb_info<MenuObject*,int>                                                menu-object.obj                          napi_get_cb_info<MenuObject*,int>
+    4.54: __acrt_fltout                                                                    cfout.obj                                __acrt_fltout
+    4.32: convert_to_fos_high_precision<double>                                            cfout.obj                                convert_to_fos_high_precision<double>
+    3.38: UnDecorator::composeDeclaration                                                  undname.obj                              private: static class DName __cdecl UnDecorator::composeDeclaration(class DName const & __ptr64)
+    1.92: UnDecorator::getDataIndirectType                                                 undname.obj                              private: static class DName __cdecl UnDecorator::getDataIndirectType(class DName const & __ptr64,char const * __ptr64,class DName const & __ptr64,int)
+    1.85: UnDecorator::getOperatorName                                                     undname.obj                              private: static class DName __cdecl UnDecorator::getOperatorName(bool,bool * __ptr64)
+    1.83: __crt_strtox::parse_integer<unsigned__int64,__crt_strtox::c_string_character_source<wchar_t>> atox.obj                                 unsigned __int64 __cdecl __crt_strtox::parse_integer<unsigned __int64,class __crt_strtox::c_string_character_source<wchar_t> >(struct __crt_locale_pointers * __ptr64 const,class __crt_strtox::c_string_character_source<wchar_t>,int,bool)
+    1.77: napi_get_cb_info<void,napi_buffer_info>                                          reg-icon-stream.obj                      enum napi_status __cdecl napi_get_cb_info<void,struct napi_buffer_info>(struct napi_env__ * __ptr64,struct napi_callback_info__ * __ptr64,void * __ptr64,void * __ptr64 * __ptr64,int,struct napi_buffer_info * __ptr64)
+    1.66: __crt_strtox::parse_integer<unsignedlong,__crt_strtox::c_string_character_source<wchar_t>> atox.obj                                 unsigned long __cdecl __crt_strtox::parse_integer<unsigned long,class __crt_strtox::c_string_character_source<wchar_t> >(struct __crt_locale_pointers * __ptr64 const,class __crt_strtox::c_string_character_source<wchar_t>,int,bool)
     1.61: Concurrency::details::UMS::Initialize                                            UMSWrapper.obj                           public: static void __cdecl Concurrency::details::UMS::Initialize(void)
-    1.58: napi_get_cb_info<void,unsignedint,icon_size_t>                                   icon-object.obj                          enum napi_status __cdecl napi_get_cb_info<void,unsigned int,struct icon_size_t>(struct napi_env__ * __ptr64,struct napi_callback_info__ * __ptr64,void * __ptr64,void * __ptr64 * __ptr64,int,unsigned int * __ptr64,struct icon_size_t * __ptr64)
-    1.58: napi_get_cb_info<void,std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t>>,icon_size_t> icon-object.obj                          enum napi_status __cdecl napi_get_cb_info<void,class std::basic_string<wchar_t,struct std::char_traits<wchar_t>,class std::allocator<wchar_t> >,struct icon_size_t>(struct napi_env__ * __ptr64,struct napi_callback_info__ * __ptr64,void * __ptr64,void * __ptr64 * __ptr64,int,class std::basic_string<wchar_t,struct std::char_traits<wchar_t>,class...
-    1.51: napi_get_cb_info<void,napi_buffer_info>                                          reg-icon-stream.obj                      enum napi_status __cdecl napi_get_cb_info<void,struct napi_buffer_info>(struct napi_env__ * __ptr64,struct napi_callback_info__ * __ptr64,void * __ptr64,void * __ptr64 * __ptr64,int,struct napi_buffer_info * __ptr64)
+    1.59: __acrt_locale_initialize_ctype                                                   initctype.obj                            __acrt_locale_initialize_ctype
+    1.51: napi_get_cb_info<MenuObject*,int,int>                                            menu-object.obj                          napi_get_cb_info<MenuObject*,int,int>
+    1.51: napi_get_cb_info<MenuObject*,int,menu_item>                                      menu-object.obj                          napi_get_cb_info<MenuObject*,int,menu_item>
+    1.50: napi_get_cb_info<NotifyIconObject*,notify_icon_object_options>                   notify-icon-object.obj                   napi_get_cb_info<NotifyIconObject*,notify_icon_object_options>
+<...>

 Object files by code size (kilobytes, min 2.00):
-   19.70: SchedulerBase.obj
-   18.92: ResourceManager.obj
-   16.66: menu-object.obj
-   10.71: icon-object.obj
-    9.83: notify-icon-object.obj
-    8.55: data.obj
-    7.52: ScheduleGroupBase.obj
-    7.44: SearchAlgorithms.obj
-    7.12: InternalContextBase.obj
-    5.83: ContextBase.obj
-    5.78: SchedulerProxy.obj
-    5.53: TaskCollection.obj
-    5.21: event.obj
-    5.21: frame.obj
-    4.13: notify-icon-message-loop.obj
-    3.99: VirtualProcessor.obj
-    3.28: excptptr.obj
-    3.16: core.obj
-    3.05: write.obj
-    2.88: rtlocks.obj
-    2.61: mbctype.obj
-    2.42: argv_wildcards.obj
-    2.31: SchedulingNode.obj
-    2.06: per_thread_data.obj
+   95.26: output.obj
+   28.33: undname.obj
+   26.39: * Linker *
+   25.89: SchedulerBase.obj
+   22.12: ResourceManager.obj
+   20.69: menu-object.obj
+   13.50: cfout.obj
+   13.17: TaskCollection.obj
+   13.16: frame.obj
+   12.33: ContextBase.obj
+   12.25: notify-icon-object.obj
+   11.61: icon-object.obj
+    9.99: ScheduleGroupBase.obj
+    9.06: data.obj
+    8.91: wsetlocale.obj
+    8.10: SearchAlgorithms.obj
+    7.81: InternalContextBase.obj
+    7.34: argv_wildcards.obj
+    7.25: atox.obj
+    6.85: winapi_thunks.obj
+    6.22: SchedulerProxy.obj
+    6.20: event.obj
+    6.19: rtlocks.obj
+    5.22: VirtualProcessor.obj
+    4.75: UMSFreeVirtualProcessorRoot.obj
+    4.69: ThreadProxyFactoryManager.obj
+    4.67: notify-icon-message-loop.obj
+<...>
 
-Overall code:    243.09 kb
-Overall data:     12.31 kb
-Overall BSS:       4.21 kb
-Overall other:     1.45 kb
+Overall code:    613.41 kb
+Overall data:     12.43 kb
+Overall BSS:       4.22 kb
+Overall other:     1.43 kb

There's a whole new bunch of VC++ de-mangling and formatting code included... huh?

I couldn't see anything on the v12.0.0 changelog that made it look like this was a deliberate change, and 600kB+/200%+ extra for each addon is pretty heavy, so I thought it was worth raising, if only to have something for people to be pointed at.

For now, I'm happy to target an earlier node version.

@simonbuchan simonbuchan changed the title VC++ on v12+ builds bloated binaries. VC++ on v12+ builds bloated addon binaries. Sep 9, 2019
@addaleax addaleax added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. addons Issues and PRs related to native addons. labels Sep 9, 2019
@addaleax
Copy link
Member

addaleax commented Sep 9, 2019

@nodejs/platform-windows

@bnoordhuis
Copy link
Member

It sounds plausible but I don't seen obvious culprits in git diff v10.16.3..v12.9.1 common.gypi (that's the file node-gyp uses.)

Is there a way for you to check what runtime your add-on links against? I could see that making a difference.

@simonbuchan
Copy link
Author

VS 2019, so it should be picking up the universal runtime, seems like both are statically linking it.

With --target=v10.16.0:

>dumpbin /nologo /dependents notify_icon.node

Dump of file notify_icon.node

File Type: DLL

  Image has the following dependencies:

    KERNEL32.dll
    USER32.dll
    ADVAPI32.dll
    SHELL32.dll

  Image has the following delay load dependencies:

    node.exe

  Summary

        5000 .data
        5000 .pdata
       17000 .rdata
        1000 .reloc
        1000 .rsrc
       3E000 .text
        1000 _RDATA

With --target=v12.0.0:

>dumpbin /nologo /dependents notify_icon.node

Dump of file notify_icon.node

File Type: DLL

  Image has the following dependencies:

    KERNEL32.dll
    USER32.dll
    ADVAPI32.dll
    SHELL32.dll

  Image has the following delay load dependencies:

    node.exe

  Summary

        1000 .00cfg
        7000 .data
        1000 .didat
        2000 .idata
        C000 .pdata
       2D000 .rdata
        2000 .reloc
        1000 .rsrc
       D1000 .text
        1000 _RDATA

And only napi_* imports on node.exe.

If it helps, here's the gyp spam for 10.16.0:

gyp info find Python using Python version 2.7.15 found at "C:\Python27\python.exe"
gyp info find VS using VS2019 (16.1.29020.237) found at:
gyp info find VS "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community"
gyp info find VS run with --verbose for detailed information
gyp info spawn C:\Python27\python.exe
gyp info spawn args [
gyp info spawn args   'C:\\code\\personal\\napi-tray\\node_modules\\node-gyp\\gyp\\gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'msvs',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\code\\personal\\napi-tray\\build\\config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\code\\personal\\napi-tray\\node_modules\\node-gyp\\addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\simon\\.node-gyp\\10.16.0\\include\\node\\common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=C:\\Users\\simon\\.node-gyp\\10.16.0',
gyp info spawn args   '-Dnode_gyp_dir=C:\\code\\personal\\napi-tray\\node_modules\\node-gyp',
gyp info spawn args   '-Dnode_lib_file=C:\\Users\\simon\\.node-gyp\\10.16.0\\<(target_arch)\\node.lib',
gyp info spawn args   '-Dmodule_root_dir=C:\\code\\personal\\napi-tray',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'C:\\code\\personal\\napi-tray\\build',
gyp info spawn args   '-Goutput_dir=.'
gyp info spawn args ]
gyp info spawn C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Current\Bin\MSBuild.exe
gyp info spawn args [
gyp info spawn args   'build/binding.sln',
gyp info spawn args   '/clp:Verbosity=minimal',
gyp info spawn args   '/nologo',
gyp info spawn args   '/p:Configuration=Release;Platform=x64'
gyp info spawn args ]

and:

gyp info find Python using Python version 2.7.15 found at "C:\Python27\python.exe"
gyp info find VS using VS2019 (16.1.29020.237) found at:
gyp info find VS "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community"
gyp info find VS run with --verbose for detailed information
gyp info spawn C:\Python27\python.exe
gyp info spawn args [
gyp info spawn args   'C:\\code\\personal\\napi-tray\\node_modules\\node-gyp\\gyp\\gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'msvs',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\code\\personal\\napi-tray\\build\\config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\code\\personal\\napi-tray\\node_modules\\node-gyp\\addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\simon\\.node-gyp\\12.0.0\\include\\node\\common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=C:\\Users\\simon\\.node-gyp\\12.0.0',
gyp info spawn args   '-Dnode_gyp_dir=C:\\code\\personal\\napi-tray\\node_modules\\node-gyp',
gyp info spawn args   '-Dnode_lib_file=C:\\Users\\simon\\.node-gyp\\12.0.0\\<(target_arch)\\node.lib',
gyp info spawn args   '-Dmodule_root_dir=C:\\code\\personal\\napi-tray',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'C:\\code\\personal\\napi-tray\\build',
gyp info spawn args   '-Goutput_dir=.'
gyp info spawn args ]
gyp info spawn C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Current\Bin\MSBuild.exe
gyp info spawn args [
gyp info spawn args   'build/binding.sln',
gyp info spawn args   '/clp:Verbosity=minimal',
gyp info spawn args   '/nologo',
gyp info spawn args   '/p:Configuration=Release;Platform=x64'
gyp info spawn args ]

doesn't seem to be anything suspicious there.

@simonbuchan
Copy link
Author

simonbuchan commented Sep 10, 2019

Ugh, should have started here, the vcxproj diff is:

--- v10.16.0.vcxproj	Tue Sep 10 18:19:55 2019
+++ v12.0.0.vcxproj	Tue Sep 10 18:20:44 2019
@@ -36,7 +36,6 @@
     <ExecutablePath>$(ExecutablePath);$(MSBuildProjectDirectory)\..\bin\;$(MSBuildProjectDirectory)\..\bin\</ExecutablePath>
     <IgnoreImportLibrary>true</IgnoreImportLibrary>
     <IntDir>$(Configuration)\obj\$(ProjectName)\</IntDir>
-    <LinkIncremental Condition="'$(Configuration)|$(Platform)'=='Release|x64'">false</LinkIncremental>
     <LinkIncremental Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</LinkIncremental>
     <OutDir>$(SolutionDir)$(Configuration)\</OutDir>
     <TargetExt Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">.node</TargetExt>
@@ -48,8 +47,8 @@
   </PropertyGroup>
   <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">
     <ClCompile>
-      <AdditionalIncludeDirectories>C:\Users\simon\.node-gyp\10.16.0\include\node;C:\Users\simon\.node-gyp\10.16.0\src;C:\Users\simon\.node-gyp\10.16.0\deps\openssl\config;C:\Users\simon\.node-gyp\10.16.0\deps\openssl\openssl\include;C:\Users\simon\.node-gyp\10.16.0\deps\uv\include;C:\Users\simon\.node-gyp\10.16.0\deps\zlib;C:\Users\simon\.node-gyp\10.16.0\deps\v8\include;..\src;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
-      <AdditionalOptions>/std:c++17 /bigobj %(AdditionalOptions)</AdditionalOptions>
+      <AdditionalIncludeDirectories>C:\Users\simon\.node-gyp\12.0.0\include\node;C:\Users\simon\.node-gyp\12.0.0\src;C:\Users\simon\.node-gyp\12.0.0\deps\openssl\config;C:\Users\simon\.node-gyp\12.0.0\deps\openssl\openssl\include;C:\Users\simon\.node-gyp\12.0.0\deps\uv\include;C:\Users\simon\.node-gyp\12.0.0\deps\zlib;C:\Users\simon\.node-gyp\12.0.0\deps\v8\include;..\src;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalOptions>/std:c++17 %(AdditionalOptions)</AdditionalOptions>
       <BasicRuntimeChecks>EnableFastChecks</BasicRuntimeChecks>
       <BufferSecurityCheck>true</BufferSecurityCheck>
       <CompileAsWinRT>false</CompileAsWinRT>
@@ -61,7 +60,7 @@
       <OmitFramePointers>false</OmitFramePointers>
       <Optimization>Disabled</Optimization>
       <PrecompiledHeader>NotUsing</PrecompiledHeader>
-      <PreprocessorDefinitions>NODE_GYP_MODULE_NAME=notify_icon;USING_UV_SHARED=1;USING_V8_SHARED=1;V8_DEPRECATION_WARNINGS=1;WIN32;_CRT_SECURE_NO_DEPRECATE;_CRT_NONSTDC_NO_DEPRECATE;_HAS_EXCEPTIONS=0;_WIN32_WINNT=_WIN32_WINNT_WIN7;_UNICODE;UNICODE;BUILDING_NODE_EXTENSION;HOST_BINARY=&quot;node.exe&quot;;DEBUG;_DEBUG;V8_ENABLE_CHECKS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
+      <PreprocessorDefinitions>NODE_GYP_MODULE_NAME=notify_icon;USING_UV_SHARED=1;USING_V8_SHARED=1;V8_DEPRECATION_WARNINGS=1;V8_DEPRECATION_WARNINGS;V8_IMMINENT_DEPRECATION_WARNINGS;WIN32;_CRT_SECURE_NO_DEPRECATE;_CRT_NONSTDC_NO_DEPRECATE;_HAS_EXCEPTIONS=0;OPENSSL_THREADS;_WIN32_WINNT=_WIN32_WINNT_WIN7;_UNICODE;UNICODE;BUILDING_NODE_EXTENSION;HOST_BINARY=&quot;node.exe&quot;;DEBUG;_DEBUG;V8_ENABLE_CHECKS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary>
       <StringPooling>true</StringPooling>
       <SuppressStartupBanner>true</SuppressStartupBanner>
@@ -69,28 +68,23 @@
       <WarningLevel>Level4</WarningLevel>
     </ClCompile>
     <Link>
-      <AdditionalDependencies>kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;DelayImp.lib;&quot;C:\Users\simon\.node-gyp\10.16.0\x64\node.lib&quot;</AdditionalDependencies>
+      <AdditionalDependencies>kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;DelayImp.lib;&quot;C:\Users\simon\.node-gyp\12.0.0\x64\node.lib&quot;</AdditionalDependencies>
       <AdditionalOptions>/ignore:4199 %(AdditionalOptions)</AdditionalOptions>
-      <AllowIsolation>true</AllowIsolation>
-      <DataExecutionPrevention>true</DataExecutionPrevention>
       <DelayLoadDLLs>node.exe;%(DelayLoadDLLs)</DelayLoadDLLs>
       <GenerateDebugInformation>true</GenerateDebugInformation>
-      <GenerateMapFile>true</GenerateMapFile>
-      <MapExports>true</MapExports>
       <OutputFile>$(OutDir)$(ProjectName).node</OutputFile>
-      <RandomizedBaseAddress>true</RandomizedBaseAddress>
       <SuppressStartupBanner>true</SuppressStartupBanner>
       <TargetExt>.node</TargetExt>
       <TargetMachine>MachineX64</TargetMachine>
     </Link>
     <ResourceCompile>
-      <AdditionalIncludeDirectories>C:\Users\simon\.node-gyp\10.16.0\include\node;C:\Users\simon\.node-gyp\10.16.0\src;C:\Users\simon\.node-gyp\10.16.0\deps\openssl\config;C:\Users\simon\.node-gyp\10.16.0\deps\openssl\openssl\include;C:\Users\simon\.node-gyp\10.16.0\deps\uv\include;C:\Users\simon\.node-gyp\10.16.0\deps\zlib;C:\Users\simon\.node-gyp\10.16.0\deps\v8\include;..\src;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
-      <PreprocessorDefinitions>NODE_GYP_MODULE_NAME=notify_icon;USING_UV_SHARED=1;USING_V8_SHARED=1;V8_DEPRECATION_WARNINGS=1;WIN32;_CRT_SECURE_NO_DEPRECATE;_CRT_NONSTDC_NO_DEPRECATE;_HAS_EXCEPTIONS=0;_WIN32_WINNT=_WIN32_WINNT_WIN7;_UNICODE;UNICODE;BUILDING_NODE_EXTENSION;HOST_BINARY=&quot;node.exe&quot;;DEBUG;_DEBUG;V8_ENABLE_CHECKS;%(PreprocessorDefinitions);%(PreprocessorDefinitions)</PreprocessorDefinitions>
+      <AdditionalIncludeDirectories>C:\Users\simon\.node-gyp\12.0.0\include\node;C:\Users\simon\.node-gyp\12.0.0\src;C:\Users\simon\.node-gyp\12.0.0\deps\openssl\config;C:\Users\simon\.node-gyp\12.0.0\deps\openssl\openssl\include;C:\Users\simon\.node-gyp\12.0.0\deps\uv\include;C:\Users\simon\.node-gyp\12.0.0\deps\zlib;C:\Users\simon\.node-gyp\12.0.0\deps\v8\include;..\src;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <PreprocessorDefinitions>NODE_GYP_MODULE_NAME=notify_icon;USING_UV_SHARED=1;USING_V8_SHARED=1;V8_DEPRECATION_WARNINGS=1;V8_DEPRECATION_WARNINGS;V8_IMMINENT_DEPRECATION_WARNINGS;WIN32;_CRT_SECURE_NO_DEPRECATE;_CRT_NONSTDC_NO_DEPRECATE;_HAS_EXCEPTIONS=0;OPENSSL_THREADS;_WIN32_WINNT=_WIN32_WINNT_WIN7;_UNICODE;UNICODE;BUILDING_NODE_EXTENSION;HOST_BINARY=&quot;node.exe&quot;;DEBUG;_DEBUG;V8_ENABLE_CHECKS;%(PreprocessorDefinitions);%(PreprocessorDefinitions)</PreprocessorDefinitions>
     </ResourceCompile>
   </ItemDefinitionGroup>
   <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
     <ClCompile>
-      <AdditionalIncludeDirectories>C:\Users\simon\.node-gyp\10.16.0\include\node;C:\Users\simon\.node-gyp\10.16.0\src;C:\Users\simon\.node-gyp\10.16.0\deps\openssl\config;C:\Users\simon\.node-gyp\10.16.0\deps\openssl\openssl\include;C:\Users\simon\.node-gyp\10.16.0\deps\uv\include;C:\Users\simon\.node-gyp\10.16.0\deps\zlib;C:\Users\simon\.node-gyp\10.16.0\deps\v8\include;..\src;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories>C:\Users\simon\.node-gyp\12.0.0\include\node;C:\Users\simon\.node-gyp\12.0.0\src;C:\Users\simon\.node-gyp\12.0.0\deps\openssl\config;C:\Users\simon\.node-gyp\12.0.0\deps\openssl\openssl\include;C:\Users\simon\.node-gyp\12.0.0\deps\uv\include;C:\Users\simon\.node-gyp\12.0.0\deps\zlib;C:\Users\simon\.node-gyp\12.0.0\deps\v8\include;..\src;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <AdditionalOptions>/std:c++17 %(AdditionalOptions)</AdditionalOptions>
       <BufferSecurityCheck>true</BufferSecurityCheck>
       <CompileAsWinRT>false</CompileAsWinRT>
@@ -105,38 +99,27 @@
       <OmitFramePointers>true</OmitFramePointers>
       <Optimization>Full</Optimization>
       <PrecompiledHeader>NotUsing</PrecompiledHeader>
-      <PreprocessorDefinitions>NODE_GYP_MODULE_NAME=notify_icon;USING_UV_SHARED=1;USING_V8_SHARED=1;V8_DEPRECATION_WARNINGS=1;WIN32;_CRT_SECURE_NO_DEPRECATE;_CRT_NONSTDC_NO_DEPRECATE;_HAS_EXCEPTIONS=0;_WIN32_WINNT=_WIN32_WINNT_WIN7;_UNICODE;UNICODE;BUILDING_NODE_EXTENSION;HOST_BINARY=&quot;node.exe&quot;;%(PreprocessorDefinitions)</PreprocessorDefinitions>
+      <PreprocessorDefinitions>NODE_GYP_MODULE_NAME=notify_icon;USING_UV_SHARED=1;USING_V8_SHARED=1;V8_DEPRECATION_WARNINGS=1;V8_DEPRECATION_WARNINGS;V8_IMMINENT_DEPRECATION_WARNINGS;WIN32;_CRT_SECURE_NO_DEPRECATE;_CRT_NONSTDC_NO_DEPRECATE;_HAS_EXCEPTIONS=0;OPENSSL_THREADS;_WIN32_WINNT=_WIN32_WINNT_WIN7;_UNICODE;UNICODE;BUILDING_NODE_EXTENSION;HOST_BINARY=&quot;node.exe&quot;;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreaded</RuntimeLibrary>
       <RuntimeTypeInfo>false</RuntimeTypeInfo>
       <StringPooling>true</StringPooling>
       <SuppressStartupBanner>true</SuppressStartupBanner>
       <TreatWarningAsError>false</TreatWarningAsError>
       <WarningLevel>Level4</WarningLevel>
-      <WholeProgramOptimization>true</WholeProgramOptimization>
     </ClCompile>
-    <Lib>
-      <AdditionalOptions>/LTCG:INCREMENTAL %(AdditionalOptions)</AdditionalOptions>
-    </Lib>
     <Link>
-      <AdditionalDependencies>kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;DelayImp.lib;&quot;C:\Users\simon\.node-gyp\10.16.0\x64\node.lib&quot;</AdditionalDependencies>
-      <AdditionalOptions>/ignore:4199 /LTCG:INCREMENTAL %(AdditionalOptions)</AdditionalOptions>
-      <AllowIsolation>true</AllowIsolation>
-      <DataExecutionPrevention>true</DataExecutionPrevention>
+      <AdditionalDependencies>kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;DelayImp.lib;&quot;C:\Users\simon\.node-gyp\12.0.0\x64\node.lib&quot;</AdditionalDependencies>
+      <AdditionalOptions>/ignore:4199 %(AdditionalOptions)</AdditionalOptions>
       <DelayLoadDLLs>node.exe;%(DelayLoadDLLs)</DelayLoadDLLs>
-      <EnableCOMDATFolding>true</EnableCOMDATFolding>
       <GenerateDebugInformation>true</GenerateDebugInformation>
-      <GenerateMapFile>true</GenerateMapFile>
-      <MapExports>true</MapExports>
-      <OptimizeReferences>true</OptimizeReferences>
       <OutputFile>$(OutDir)$(ProjectName).node</OutputFile>
-      <RandomizedBaseAddress>true</RandomizedBaseAddress>
       <SuppressStartupBanner>true</SuppressStartupBanner>
       <TargetExt>.node</TargetExt>
       <TargetMachine>MachineX64</TargetMachine>
     </Link>
     <ResourceCompile>
-      <AdditionalIncludeDirectories>C:\Users\simon\.node-gyp\10.16.0\include\node;C:\Users\simon\.node-gyp\10.16.0\src;C:\Users\simon\.node-gyp\10.16.0\deps\openssl\config;C:\Users\simon\.node-gyp\10.16.0\deps\openssl\openssl\include;C:\Users\simon\.node-gyp\10.16.0\deps\uv\include;C:\Users\simon\.node-gyp\10.16.0\deps\zlib;C:\Users\simon\.node-gyp\10.16.0\deps\v8\include;..\src;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
-      <PreprocessorDefinitions>NODE_GYP_MODULE_NAME=notify_icon;USING_UV_SHARED=1;USING_V8_SHARED=1;V8_DEPRECATION_WARNINGS=1;WIN32;_CRT_SECURE_NO_DEPRECATE;_CRT_NONSTDC_NO_DEPRECATE;_HAS_EXCEPTIONS=0;_WIN32_WINNT=_WIN32_WINNT_WIN7;_UNICODE;UNICODE;BUILDING_NODE_EXTENSION;HOST_BINARY=&quot;node.exe&quot;;%(PreprocessorDefinitions);%(PreprocessorDefinitions)</PreprocessorDefinitions>
+      <AdditionalIncludeDirectories>C:\Users\simon\.node-gyp\12.0.0\include\node;C:\Users\simon\.node-gyp\12.0.0\src;C:\Users\simon\.node-gyp\12.0.0\deps\openssl\config;C:\Users\simon\.node-gyp\12.0.0\deps\openssl\openssl\include;C:\Users\simon\.node-gyp\12.0.0\deps\uv\include;C:\Users\simon\.node-gyp\12.0.0\deps\zlib;C:\Users\simon\.node-gyp\12.0.0\deps\v8\include;..\src;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <PreprocessorDefinitions>NODE_GYP_MODULE_NAME=notify_icon;USING_UV_SHARED=1;USING_V8_SHARED=1;V8_DEPRECATION_WARNINGS=1;V8_DEPRECATION_WARNINGS;V8_IMMINENT_DEPRECATION_WARNINGS;WIN32;_CRT_SECURE_NO_DEPRECATE;_CRT_NONSTDC_NO_DEPRECATE;_HAS_EXCEPTIONS=0;OPENSSL_THREADS;_WIN32_WINNT=_WIN32_WINNT_WIN7;_UNICODE;UNICODE;BUILDING_NODE_EXTENSION;HOST_BINARY=&quot;node.exe&quot;;%(PreprocessorDefinitions);%(PreprocessorDefinitions)</PreprocessorDefinitions>
     </ResourceCompile>
   </ItemDefinitionGroup>
   <ItemGroup>

A bit of noise in the Debug section further up - the relevant change further down is the missing <OptimizeReferences>true</OptimizeReferences> manually adding that reverts the size down to ~300kB, but a lot of the other missing optimizations and security mitigation stuff seems like it should be restored too...

@simonbuchan
Copy link
Author

Looks like the culprit is 20a917c moving the optimization options into node.gyp while disabling LTCG by default, which it seems addons were getting by default too.

Seems like there's little reason to not at least restore the non LTCG optimizations to common.gypi, since they don't have nearly the build time impact but seem to have pretty huge code size improvements (potentially to the point of improving build time?):

'VCLinkerTool': {
  'OptimizeReferences': 2,             # /OPT:REF
  'EnableCOMDATFolding': 2,            # /OPT:ICF
  'LinkIncremental': 1,                # disable incremental linking
}

@refack, as the original committer, does the above make sense?

simonbuchan added a commit to simonbuchan/node-not-the-systray that referenced this issue Sep 10, 2019
@bnoordhuis
Copy link
Member

@nodejs/platform-windows See above. @simonbuchan's conclusion seems on point to me.

@deepak1556
Copy link
Contributor

Also reiterating my comments from #25931 (comment), any reason this configuration for native modules cannot be added to node-gyp's addon.gypi file if the original reason for moving them out was build time concerns ? This would benefit embedders like Electron who build node as a GN target and hence maintains its own generation of common.gypi, instead of fixing it in Electron I would like to have it fixed here that would benefit all. Thoughts ?

@richardlau
Copy link
Member

Putting the configuration into node-gyp's addon.gypi would be consistent with nodejs/node-gyp#1118 goal of decoupling addons from being affected by common.gypi changes (like what happened here).

@deepak1556
Copy link
Contributor

@richardlau thanks for the pointer! I can try sending a PR for the node-gyp issue as it shares my concerns as well.

@bzoz
Copy link
Contributor

bzoz commented Mar 31, 2020

Fixed by nodejs/node-gyp@e18a61a

@bzoz bzoz closed this as completed Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addons Issues and PRs related to native addons. build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants