microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.31k stars 1.14k forks source link

--singleproc flag not respected on machines with more than 16GB of memory #12181

Open sivakusayan opened 1 year ago

sivakusayan commented 1 year ago

Problem Description

As noted in this code (thanks to @ryfow for the investigation), if a computer has more than 16GB of memory and the --singleproc flag is passed, we will not respect the --singleproc flag because of the OR check.

// Building projects in parallel increases compiler memory usage and
// doesn't lead to dramatic performance gains (See #4739). Only enable
// parallel builds on machines with >16GB of memory to avoid OOM errors
const highMemory = totalmem() > 16 * 1024 * 1024 * 1024;
const enableParallelBuilds = singleproc === false || highMemory;

Steps To Reproduce

  1. Make sure you're on a computer with more than 16GB of memory.
  2. Run the following commands in the directory of your choice:
    npx react-native init test
    cd test
    npx react-native-windows-init
    yarn windows --singleproc --logging
  3. Note that the argument /maxCpuCount is logged when we log the arguments passed to MSBuild.

Expected Results

I expect that /maxCpuCount is not passed to MSBuild when the --singleproc flag is passed.

CLI version

11.3.7

Environment

System:
  OS: Windows 10 10.0.19045
  CPU: (16) x64 11th Gen Intel(R) Core(TM) i7-11850H @ 2.50GHz
  Memory: 19.97 GB / 31.73 GB
Binaries:
  Node:
    version: 18.18.0
    path: C:\Program Files\nodejs\node.EXE
  Yarn:
    version: 1.22.19
    path: ~\AppData\Roaming\npm\yarn.CMD
  npm:
    version: 9.8.1
    path: C:\Program Files\nodejs\npm.CMD
  Watchman: Not Found
SDKs:
  Android SDK: Not Found
  Windows SDK:
    AllowDevelopmentWithoutDevLicense: Enabled
    AllowAllTrustedApps: Enabled
    Versions:
      - 10.0.19041.0
      - 10.0.22621.0
IDEs:
  Android Studio: Not Found
  Visual Studio:
    - 17.7.34031.279 (Visual Studio Professional 2022)
Languages:
  Java: Not Found
  Ruby: Not Found
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.72.5
    wanted: 0.72.5
  react-native-windows:
    installed: 0.72.9
    wanted: 0.72.9
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: Not found
  newArchEnabled: Not found

Target Platform Version

10.0.19041

Target Device(s)

Desktop

Visual Studio Version

Visual Studio 2022

Build Configuration

Debug

Snack, code example, screenshot, or link to a repository

No response

sivakusayan commented 1 year ago

In case people are wondering why we might want to force single process builds, we're getting some errors from MSBuild processes tripping each other up by trying to get locks on the same file, resulting in build failures.

There might be a simpler solution for what we need (I'm not too familiar with the MSBuild ecosystem) but for now we're probably just going to roll with a naked msbuild command instead of using the react-native-windows wrapper around msbuild.

chrisglein commented 1 year ago

Should be a supported arg, not sure why it's not making it through to msbuild. Was added in this PR: https://github.com/microsoft/react-native-windows/pull/5784

sivakusayan commented 1 year ago

For people who need a workaround, we used patch-package to get around this with this patch:

diff --git a/node_modules/@react-native-windows/cli/lib-commonjs/runWindows/utils/msbuildtools.js b/node_modules/@react-native-windows/cli/lib-commonjs/runWindows/utils/msbuildtools.js
index 61a29f8..0fa023a 100644
--- a/node_modules/@react-native-windows/cli/lib-commonjs/runWindows/utils/msbuildtools.js
+++ b/node_modules/@react-native-windows/cli/lib-commonjs/runWindows/utils/msbuildtools.js
@@ -102,7 +102,7 @@ class MSBuildTools {
         // doesn't lead to dramatic performance gains (See #4739). Only enable
         // parallel builds on machines with >16GB of memory to avoid OOM errors
         const highMemory = (0, os_1.totalmem)() > 16 * 1024 * 1024 * 1024;
-        const enableParallelBuilds = singleproc === false || highMemory;
+        const enableParallelBuilds = singleproc !== undefined ? !singleproc : highMemory;
         if (enableParallelBuilds) {
             args.push('/maxCpuCount');
         }

Using git-blame, I think this is a regression from https://github.com/microsoft/react-native-windows/pull/5796.