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

Visual Studio generator (vsxxxx) doesn't allow parallel building #1062

Closed
SirLynix opened this issue Nov 23, 2020 · 12 comments
Closed

Visual Studio generator (vsxxxx) doesn't allow parallel building #1062

SirLynix opened this issue Nov 23, 2020 · 12 comments
Milestone

Comments

@SirLynix
Copy link
Member

SirLynix commented Nov 23, 2020

Describe the bug

Hi, when using the old Visual Studio 2019 generator, it outputs project files that don't allow multiprocessorcompilation, so I made a change to add them, but found out that it was still blocked because of the ObjectFileName conditions.

It does generate this:

    <ClCompile Include="..\..\..\src\CoreLib\PlayerCommandStore.cpp">
      <AdditionalOptions Condition="'$(Configuration)'=='release'">-fp:fast -fp:fast %(AdditionalOptions)</AdditionalOptions>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\.objs\CoreLib\windows\x64\release\src\CoreLib\PlayerCommandStore.cpp.obj</ObjectFileName>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='debug|x64'">..\..\.objs\CoreLib\windows\x64\debug\src\CoreLib\PlayerCommandStore.cpp.obj</ObjectFileName>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='release|Win32'">..\..\.objs\CoreLib\windows\x86\release\src\CoreLib\PlayerCommandStore.cpp.obj</ObjectFileName>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='debug|Win32'">..\..\.objs\CoreLib\windows\x86\debug\src\CoreLib\PlayerCommandStore.cpp.obj</ObjectFileName>
    </ClCompile>
    <ClCompile Include="..\..\..\src\CoreLib\PlayerMovementController.cpp">
      <AdditionalOptions Condition="'$(Configuration)'=='release'">-fp:fast -fp:fast %(AdditionalOptions)</AdditionalOptions>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='debug|Win32'">..\..\.objs\CoreLib\windows\x86\debug\src\CoreLib\PlayerMovementController.cpp.obj</ObjectFileName>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\.objs\CoreLib\windows\x64\release\src\CoreLib\PlayerMovementController.cpp.obj</ObjectFileName>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='debug|x64'">..\..\.objs\CoreLib\windows\x64\debug\src\CoreLib\PlayerMovementController.cpp.obj</ObjectFileName>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='release|Win32'">..\..\.objs\CoreLib\windows\x86\release\src\CoreLib\PlayerMovementController.cpp.obj</ObjectFileName>
    </ClCompile>
    <ClCompile Include="..\..\..\src\CoreLib\PropertyValues.cpp">
      <AdditionalOptions Condition="'$(Configuration)'=='release'">-fp:fast -fp:fast %(AdditionalOptions)</AdditionalOptions>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='release|Win32'">..\..\.objs\CoreLib\windows\x86\release\src\CoreLib\PropertyValues.cpp.obj</ObjectFileName>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='debug|Win32'">..\..\.objs\CoreLib\windows\x86\debug\src\CoreLib\PropertyValues.cpp.obj</ObjectFileName>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\.objs\CoreLib\windows\x64\release\src\CoreLib\PropertyValues.cpp.obj</ObjectFileName>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='debug|x64'">..\..\.objs\CoreLib\windows\x64\debug\src\CoreLib\PropertyValues.cpp.obj</ObjectFileName>
    </ClCompile>

Removing the ObjectFileName entry does fix and allows parallel compilation. The object files are still generated in the right folder (so I guess this isn't necessary).

Now I know the old Visual Studio generator is deprecated, but I'm having troubles with the new one, like using the "build solution" button which does something like this:

2>$xmake config -P . -p windows -m debug -a x64 -o "build"
3>$xmake config -P . -p windows -m debug -a x64 -o "build"
4>$xmake config -P . -p windows -m debug -a x64 -o "build"
1>$xmake config -P . -p windows -m debug -a x64 -o "build"
5>$xmake config -P . -p windows -m debug -a x64 -o "build"
6>$xmake config -P . -p windows -m debug -a x64 -o "build"
7>$xmake config -P . -p windows -m debug -a x64 -o "build"
2>checking for Microsoft Visual Studio (x64) version ... 2019
2>checking for Qt SDK directory ... C:/Projets/Libs/Qt/5.15.0/msvc2019_64
2>checking for Qt SDK version ... 5.15.0
2>$xmake build -P . -w BurgWarServer
1>$xmake build -P . -w lua
3>$xmake build -P . -w CoreLib
4>$xmake build -P . -w Main
5>$xmake build -P . -w BurgWarMapEditor
6>$xmake build -P . -w ClientLib
7>$xmake build -P . -w BurgWar
2>[ 27%]: compiling.debug contrib\lua\src\ltablib.c
2>[ 13%]: compiling.debug contrib\lua\src\ldo.c
2>[  7%]: compiling.debug contrib\lua\src\lauxlib.c
2>[ 23%]: compiling.debug contrib\lua\src\lparser.c
...

Which I'm guessing is troublesome, I have some files that get compiled multiple times when doing so.

Why is the VS2019 generator deprecated btw? It seems to be fine, except for that multiprocessor issue.

Expected behavior

To generate a project allowing for multiprocessor compilation.

Related Environment

Please provide compiling and running environment information:

  • xmake version: xmake v2.3.8+202011171119
  • os: Windows 10 2004
  • target platform: Windows

Additional context

I added

    -- Allow multi processor compilation
    if flagstr:find("[%-/]Gm-") or not flagstr:find("[%-/]Gm") then
        vcxprojfile:print("<MinimalRebuild%s>false</MinimalRebuild>", condition)
        vcxprojfile:print("<MultiProcessorCompilation%s>true</MultiProcessorCompilation>", condition)
    end

in the vs201x_vcxproj.lua (before -- make AdditionalIncludeDirectories) to setup correctly the multiprocessor flags, and excluded "Gm-" and "Gm" from the additional flags.

@waruqi
Copy link
Member

waruqi commented Nov 24, 2020

Why is the VS2019 generator deprecated btw? It seems to be fine, except for that multiprocessor issue.

It has not been deprecated, and users can still use it. However, the vs201x plugin cannot fully support all the features in xmake.lua, such as on_xxx/after_xxx scripts and on_xxx scripts in some built-in rules.

in the vs201x_vcxproj.lua (before -- make AdditionalIncludeDirectories) to setup correctly the multiprocessor flags, and excluded "Gm-" and "Gm" from the additional flags.

MultiProcessorCompilation -> enable /MP? I think the /MP flags should be configured in xmake.lua to decide to let vcproj support parallel compilation.

Removing the ObjectFileName entry does fix and allows parallel compilation. The object files are still generated in the right folder (so I guess this isn't necessary).

Why? If we don't tell it, how does vs know the real output path of each object file?

Now I know the old Visual Studio generator is deprecated, but I'm having troubles with the new one, like using the "build solution" button which does something like this:
Which I'm guessing is troublesome, I have some files that get compiled multiple times when doing so.

This just looks strange, but there is no problem. Each source file should only be compiled once.

Xmake will have a file lock to ensure that each target is compiled one by one. If the current source file has been compiled, it will not be compiled again.

Another solution is that you can add a target("all") to associate all other targets, and then just choose to compile this all target

target("all")
    set_kind("phony")
    add_deps("t1", "t2", "t3")

@SirLynix
Copy link
Member Author

SirLynix commented Nov 24, 2020

It has not been deprecated, and users can still use it. However, the vs201x plugin cannot fully support all the features in xmake.lua, such as on_xxx/after_xxx scripts and on_xxx scripts in some built-in rules.

Fair enough, so as long as I don't use this it's fine to use the vs2019 generator?

MultiProcessorCompilation -> enable /MP? I think the /MP flags should be configured in xmake.lua to decide to let vcproj support parallel compilation.

It's possible, but since minimal regeneration is deprecated, it could also be enabled by default imo.

Why? If we don't tell it, how does vs know the real output path of each object file?

It doesn't have to be set for each file, in the .vcxproj file there's already

  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='debug|Win32'">
    <OutDir>..\..\windows\x86\debug\</OutDir>
    <IntDir>..\..\.objs\CoreLib\windows\x86\debug\</IntDir>
    <TargetName>CoreLib</TargetName>
    <TargetExt>.lib</TargetExt>
  </PropertyGroup>
  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='debug|x64'">
    <OutDir>..\..\windows\x64\debug\</OutDir>
    <IntDir>..\..\.objs\CoreLib\windows\x64\debug\</IntDir>
    <TargetName>CoreLib</TargetName>
    <TargetExt>.lib</TargetExt>
  </PropertyGroup>
  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='release|Win32'">
    <OutDir>..\..\windows\x86\release\</OutDir>
    <IntDir>..\..\.objs\CoreLib\windows\x86\release\</IntDir>
    <TargetName>CoreLib</TargetName>
    <TargetExt>.lib</TargetExt>
  </PropertyGroup>

which does the trick.

This just looks strange, but there is no problem. Each source file should only be compiled once.

Xmake will have a file lock to ensure that each target is compiled one by one. If the current source file has been compiled, it will not be compiled again.

I rechecked on this and you're right, files get only compiled once, I had multiple compilations of the same file because one of my target failed to compile, nvm. It's interesting to know about this file lock!

The phony all solution is also quite interesting, I think cmake does something like this.

@waruqi
Copy link
Member

waruqi commented Nov 24, 2020

Fair enough, so as long as I don't use this it's fine to use the vs2019 generator?

Yes, as long as you don’t use on_xxx scripts or some rules with on_xxx scripts, such as qt projects (with qt.qrc rules)

It's possible, but since minimal regeneration is deprecated, it could also be enabled by default imo.

But there are still some other flags that are not compatible with it. I think it is better for users to manually enable it in VS.

https://docs.microsoft.com/en-us/cpp/build/reference/mp-build-with-multiple-processes?view=msvc-160

-- --
/E, /EP Copies preprocessor output to the standard output (stdout).
/Gm Deprecated. Enables an incremental rebuild.
/showIncludes Writes a list of include files to the standard error (stderr).
/Yc Writes a precompiled header file.

It doesn't have to be set for each file, in the .vcxproj file there's already

You are right, here I can improve it. Thank you.

@SirLynix
Copy link
Member Author

No problem, do you want me to make a PR to handle /Gm- /Gm and /MP flags?

@waruqi
Copy link
Member

waruqi commented Nov 24, 2020

Removing the ObjectFileName entry does fix and allows parallel compilation. The object files are still generated in the right folder (so I guess this isn't necessary).
It doesn't have to be set for each file, in the .vcxproj file there's already

I think there are still some other problems. If there are multiple object files with the same name in the same object root directory, will it cause conflicts?

e.g.

<IntDir>..\..\.objs\CoreLib\windows\x86\debug\</IntDir>
..\..\.objs\CoreLib\windows\x86\debug\xxx\foo.c
..\..\.objs\CoreLib\windows\x86\debug\xxx\yyy\foo.c
..\..\.objs\CoreLib\windows\x86\debug\xxx2\foo.c
..\..\.objs\CoreLib\windows\x86\debug\xxx3\foo.c
..\..\.objs\CoreLib\windows\x86\debug\xxx4\foo.c

Will VS write these object files to the same location? ..\..\.objs\CoreLib\windows\x86\debug\foo.obj?

@SirLynix
Copy link
Member Author

I just checked and folders are kept, no problem for that.
image

@waruqi
Copy link
Member

waruqi commented Nov 24, 2020

I just checked and folders are kept, no problem for that.
image

Got it, then there is no problem. I have removed ObjectFileName on dev branch.

@waruqi
Copy link
Member

waruqi commented Nov 24, 2020

No problem, do you want me to make a PR to handle /Gm- /Gm and /MP flags?

Yes, I welcome your contribution. But I still don't know how to better support it. If you use add_cxflags("/MP") to enable it, but this will also interrupt the compilation of xmake, because xmake build requires /showIncludes.

In addition, we also need to consider how to deal with /Yc flags.

Perhaps it would be better for users to modify and enable it directly in the VS project configuration property page.

@waruqi waruqi added this to the v2.3.9 milestone Nov 24, 2020
@SirLynix
Copy link
Member Author

Yes, I welcome your contribution. But I still don't know how to better support it. If you use add_cxflags("/MP") to enable it, but this will also interrupt the compilation of xmake, because xmake build requires /showIncludes.

I suppose this is only a problem when xmake is used for compilation (instead of Visual Studio), I think /MP can be dropped in that case, as xmake will do parallel compilation on its own.

@waruqi
Copy link
Member

waruqi commented Nov 24, 2020

I suppose this is only a problem when xmake is used for compilation (instead of Visual Studio), I think /MP can be dropped in that case, as xmake will do parallel compilation on its own.

You can try the following command to enable it only for vs project.

xmake f --cxflags="/MP"
xmake project -k vs 

@waruqi waruqi closed this as completed Nov 25, 2020
@waruqi
Copy link
Member

waruqi commented Jan 28, 2022

I just checked and folders are kept, no problem for that. image

I tried it, It does not work. object files will conflict. see #2016

├── src
│   ├── api
│   │   └── render.c
│   ├── main.cpp
│   └── render.c

image

@waruqi
Copy link
Member

waruqi commented Jan 29, 2022

I have fixed it.

  <ItemGroup>
    <ClCompile Include="..\..\src\main.cpp">
      <AdditionalOptions>/EHsc %(AdditionalOptions)</AdditionalOptions>
    </ClCompile>
    <ClCompile Include="..\..\src\render.c">
    </ClCompile>
    <ClCompile Include="..\..\src\api\render.c">
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='debug|Win32'">..\..\build\.objs\test\windows\x86\debug\src\api\render.c.obj</ObjectFileName>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='debug|x64'">..\..\build\.objs\test\windows\x64\debug\src\api\render.c.obj</ObjectFileName>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='release|Win32'">..\..\build\.objs\test\windows\x86\release\src\api\render.c.obj</ObjectFileName>
      <ObjectFileName Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\build\.objs\test\windows\x64\release\src\api\render.c.obj</ObjectFileName>
    </ClCompile>
  </ItemGroup>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants