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.

Sign up for GitHub

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

Jump to bottom

Optimize the size for tsbuildinfo #43155

Merged
merged 7 commits into from
Mar 10, 2021
Merged

Optimize the size for tsbuildinfo #43155

merged 7 commits into from
Mar 10, 2021

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented Mar 9, 2021 •
edited
Loading

This is simplified version of #43079 and additional optimizations for tsbuildinfo size

Changes are:

  1. fileNames : new entry which is list of filenames tsbuildinfo will encounter, index of the file name is used as fileId everywhere
  2. fileInfos: is now a list instead of map. FileFnfo is for the file at given fileId as index
  3. Semantic diagnostics and affectedFilesPendingEmit also use fileId instead of file name
  4. fileIdsList: is a list of fileIds that referenceMap or exportedModulesMap will use. Index of this list will be used as fileIdsListId for reference map
  5. referenceMap and exportedModulesMap is converted to tuple array with first entry being fileId and second one being fileIdsListId
  6. Instead of emitting tsbuildinfo with 2 space indent, its emitted without spaces
  7. Testrunner emits the old format of readable tsbuildinfo as a readable.baseline.txt so that we can verify correctness easily

@sheetalkamat
Baseline readable buildinfo
cd71acc
@sheetalkamat
Use file names as index in file name list
bda4556 
This is extension of the idea given by @sokra to optimize size of tsbuildinfo
@sheetalkamat
Deduplicate reference map lists and use file name index to sort them
2a0224e 
Different implementation of #43079 based on idea suggested by @sokra
@sheetalkamat
Minimal json.stringify for the tsbuildinfo
41026d7 
Again implementaion of suggestion by @sokra
@sheetalkamat
Copy link
Member Author

@typescript-bot pack this

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Mar 9, 2021
@sheetalkamat
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 9, 2021 •
edited
Loading

Heya @sheetalkamat, I've started to run the tarball bundle task on this PR at 41026d7. You can monitor the build here.

@sheetalkamat sheetalkamat mentioned this pull request Mar 9, 2021
Closed
4 tasks
@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 9, 2021 •
edited
Loading

Hey @sheetalkamat, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/97992/artifacts?artifactName=tgz&fileId=CCB541D9CC318BAC74920143D5E2DF0C64321E312E920809A6FF415858B299A602&fileName=/typescript-4.3.0-insiders.20210309.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

sokra
sokra approved these changes Mar 9, 2021
View reviewed changes
Copy link
Contributor

@sokra sokra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I noticed a few minor nitpicks for the test runner

@sokra
Copy link
Contributor

sokra commented Mar 9, 2021

I tried this version in my project.

Total Size: 14.9MB -> 1.0MB
Gzipped size: 526KB -> 251KB

  • fileNames size: 134KB
  • fileInfos size: 547KB
  • fileNamesList size: 283KB
  • referencedMap size: 32KB
  • exportedModulesMap size: 60KB

amcasey
amcasey reviewed Mar 9, 2021
View reviewed changes
Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please summarize the high-level changes? Pulling out numeric identifiers seems like an easy win - what else changed?

@sheetalkamat sheetalkamat requested a review from amcasey March 9, 2021 21:16
amcasey
amcasey approved these changes Mar 9, 2021
View reviewed changes
Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My review was somewhat shallow, but the explanation made sense and I didn't notice obvious bugs.

@sheetalkamat sheetalkamat merged commit 6e4456b into master Mar 10, 2021
@sheetalkamat sheetalkamat deleted the miniBuildInfo branch March 10, 2021 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sokra sokra sokra approved these changes

@amcasey amcasey amcasey approved these changes

@andrewbranch andrewbranch Awaiting requested review from andrewbranch

@RyanCavanaugh RyanCavanaugh Awaiting requested review from RyanCavanaugh

@sandersn sandersn Awaiting requested review from sandersn

Assignees

@sheetalkamat sheetalkamat

Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sheetalkamat @typescript-bot @sokra @amcasey