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

feat: big-endian support #3669

Merged
merged 36 commits into from
Jul 13, 2022
Merged

feat: big-endian support #3669

merged 36 commits into from
Jul 13, 2022

Conversation

paul-soporan
Copy link
Member

@paul-soporan paul-soporan commented Oct 28, 2021

What's the problem this PR addresses?

This PR adds big-endian support to Yarn.

Closes #2745.

How did you fix it?

Changes required to get Yarn to work on big-endian systems:

  • Recompiling libzip with -s SUPPORT_BIG_ENDIAN=1
  • Lazy-import ink everywhere and throw an error stating that interactive commands aren't supported on BE systems (Recompiling yoga-layout-prebuilt with -s SUPPORT_BIG_ENDIAN=1 Recompiling yoga-layout-prebuilt requires recompiling yoga-layout whose last release was 2 years ago. Unfortunately, compiling yoga is a nightmare because of changes in newer versions of node, clang, and possibly emscripten.)

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@arcanis
Copy link
Member

arcanis commented Oct 29, 2021

What's the impact on perfs for little endian systems?

@paul-soporan
Copy link
Member Author

The perf of the pnp hook seems to be approximately the same, I'm getting mixed results for some reason (master doesn't always beat this branch, but I'm not sure where that variation comes from):

const fs = require(`fs/promises`);

const readManifest = async archivePath => {
  let path = archivePath, listing;
  while ((listing = await fs.readdir(path))) {
    if (listing.includes(`package.json`))
      return fs.readFile(`${path}/package.json`, `utf8`);

    if (listing.length > 1)
      throw new Error(`Assertion failed`);

    path = `${path}/${listing[0]}`;
  }

  throw new Error(`Assertion failed`);
};

(async () => {
  for (let i = 0; i < 10; ++i) {
    for (const file of await fs.readdir(`./.yarn/cache`)) {
      if (file.endsWith(`.zip`)) {
        await readManifest(`./.yarn/cache/${file}`);
      }
    }
  }
})();

Master:

➜  berry git:(master) ✗ hyperfine -w 1 'node -r ./.pnp.cjs test.js'
Benchmark #1: node -r ./.pnp.cjs test.js
  Time (mean ± σ):      7.026 s ±  0.132 s    [User: 7.312 s, System: 0.802 s]
  Range (min … max):    6.745 s …  7.184 s    10 runs

This PR:

➜  berry git:(paul/feat/big-endian-suport) ✗ hyperfine -w 1 'node -r ./.pnp.cjs test.js'
Benchmark #1: node -r ./.pnp.cjs test.js
  Time (mean ± σ):      7.045 s ±  0.131 s    [User: 7.326 s, System: 0.800 s]
  Range (min … max):    6.872 s …  7.244 s    10 runs

Some tests were using a timeout of 45000 so I bumped default LE timeout to 45000 too so that we don't have to special case BE and LE everywhere.
@namrata-ibm
Copy link

@paul-soporan Having this merged would be really great! Could you please help?

@paul-soporan paul-soporan added the infra: pending update A bot will merge master into this PR label Apr 18, 2022
@github-actions github-actions bot removed the infra: pending update A bot will merge master into this PR label Apr 18, 2022
@paul-soporan paul-soporan marked this pull request as ready for review April 19, 2022 21:43
@paul-soporan paul-soporan requested a review from arcanis as a code owner April 19, 2022 21:43
@paul-soporan paul-soporan added the infra: pending update A bot will merge master into this PR label May 4, 2022
@github-actions github-actions bot removed the infra: pending update A bot will merge master into this PR label May 4, 2022
@namrata-ibm
Copy link

@paul-soporan @arcanis gentle reminder!

@chrandgull
Copy link

chrandgull commented May 17, 2022

@paul-soporan @arcanis gentle reminder!

A simple 'thank you' from the mainframe community for developing this patch. :)

I was able to build Grafana v8.5.0 with this. Great work!

@paul-soporan
Copy link
Member Author

Everything on my side should be finished other than the occasional rebase, just waiting on a review now (CC @arcanis)

@paul-soporan paul-soporan added the infra: pending update A bot will merge master into this PR label Jun 22, 2022
@github-actions github-actions bot removed the infra: pending update A bot will merge master into this PR label Jun 22, 2022
@merceyz merceyz mentioned this pull request Jul 7, 2022
2 tasks
@merceyz merceyz added the infra: pending update A bot will merge master into this PR label Jul 13, 2022
@github-actions github-actions bot removed the infra: pending update A bot will merge master into this PR label Jul 13, 2022
merceyz
merceyz previously approved these changes Jul 13, 2022
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.

[Feature] Big Endian Support
6 participants