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

refactor: optimize wireformat size #4502

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

refactor: optimize wireformat size #4502

wants to merge 6 commits into from

Conversation

seia-soto
Copy link
Member

This improves binary size by

  • Replacing 4 boolean values in resources into single byte unsigned integer
  • Limiting a space for encoded preprocessor condition to ascii

@seia-soto seia-soto requested a review from remusao as a code owner December 9, 2024 04:14
@seia-soto seia-soto self-assigned this Dec 9, 2024
@seia-soto seia-soto added the PR: Internal 🏠 Changes only affect internals label Dec 9, 2024
@seia-soto
Copy link
Member Author 

> (6749933 / 6750905) * 100
99.98560193040785

master=seia-soto:resources-checking, d4281c5

aa@MacBookPro adblocker % yarn tsx ./tools/engine-size.ts 
> ads (49534 network + 38171 hide + 0 resources)
 + raw 3358745 bytes
 + gzip 1783830 bytes
 + brotli 1467652 bytes
> ads (49534 network + 38171 hide + 136 resources)
 + raw 3472745 bytes
 + gzip 1822569 bytes
 + brotli 1497982 bytes
> ads (0 network + 38171 hide + 0 resources)
 + raw 1825421 bytes
 + gzip 880805 bytes
 + brotli 708974 bytes
> ads (0 network + 38171 hide + 136 resources)
 + raw 1939421 bytes
 + gzip 913308 bytes
 + brotli 739998 bytes
> ads (49534 network + 0 hide + 0 resources)
 + raw 1533629 bytes
 + gzip 905016 bytes
 + brotli 765458 bytes
> ads (49534 network + 0 hide + 136 resources)
 + raw 1647629 bytes
 + gzip 940601 bytes
 + brotli 795191 bytes
> ads + trackers (102405 network + 38324 hide + 0 resources)
 + raw 4958193 bytes
 + gzip 2635039 bytes
 + brotli 2172546 bytes
> ads + trackers (102405 network + 38324 hide + 136 resources)
 + raw 5072193 bytes
 + gzip 2672081 bytes
 + brotli 2205015 bytes
> ads + trackers (0 network + 38324 hide + 0 resources)
 + raw 1847893 bytes
 + gzip 890432 bytes
 + brotli 715577 bytes
> ads + trackers (0 network + 38324 hide + 136 resources)
 + raw 1961893 bytes
 + gzip 925091 bytes
 + brotli 744756 bytes
> ads + trackers (102405 network + 0 hide + 0 resources)
 + raw 3110601 bytes
 + gzip 1745248 bytes
 + brotli 1459959 bytes
> ads + trackers (102405 network + 0 hide + 136 resources)
 + raw 3224601 bytes
 + gzip 1781281 bytes
 + brotli 1491619 bytes
> ads + trackers + annoyances (104585 network + 69043 hide + 0 resources)
 + raw 6635933 bytes
 + gzip 3468626 bytes
 + brotli 2862312 bytes
> ads + trackers + annoyances (104585 network + 69043 hide + 136 resources)
 + raw 6749933 bytes
 + gzip 3501923 bytes
 + brotli 2890930 bytes
> ads + trackers + annoyances (0 network + 69043 hide + 0 resources)
 + raw 3455897 bytes
 + gzip 1674036 bytes
 + brotli 1363518 bytes
> ads + trackers + annoyances (0 network + 69043 hide + 136 resources)
 + raw 3569897 bytes
 + gzip 1708110 bytes
 + brotli 1393770 bytes
> ads + trackers + annoyances (104585 network + 0 hide + 0 resources)
 + raw 3180345 bytes
 + gzip 1793551 bytes
 + brotli 1501761 bytes
> ads + trackers + annoyances (104585 network + 0 hide + 136 resources)
 + raw 3294345 bytes
 + gzip 1829535 bytes
 + brotli 1531508 bytes

seia-soto:optimize-wireformat-0, 64287d0

aa@MacBookPro adblocker % yarn tsx ./tools/engine-size.ts 
> ads (49514 network + 38158 hide + 0 resources)
 + raw 3358885 bytes
 + gzip 1783220 bytes
 + brotli 1468075 bytes
> ads (49514 network + 38158 hide + 136 resources)
 + raw 3472617 bytes
 + gzip 1822230 bytes
 + brotli 1496937 bytes
> ads (0 network + 38158 hide + 0 resources)
 + raw 1826117 bytes
 + gzip 880925 bytes
 + brotli 706820 bytes
> ads (0 network + 38158 hide + 136 resources)
 + raw 1939849 bytes
 + gzip 913638 bytes
 + brotli 735866 bytes
> ads (49514 network + 0 hide + 0 resources)
 + raw 1533077 bytes
 + gzip 904642 bytes
 + brotli 765431 bytes
> ads (49514 network + 0 hide + 136 resources)
 + raw 1646809 bytes
 + gzip 940204 bytes
 + brotli 794984 bytes
> ads + trackers (102385 network + 38310 hide + 0 resources)
 + raw 4958161 bytes
 + gzip 2634416 bytes
 + brotli 2170333 bytes
> ads + trackers (102385 network + 38310 hide + 136 resources)
 + raw 5071893 bytes
 + gzip 2671182 bytes
 + brotli 2203007 bytes
> ads + trackers (0 network + 38310 hide + 0 resources)
 + raw 1848425 bytes
 + gzip 890172 bytes
 + brotli 716961 bytes
> ads + trackers (0 network + 38310 hide + 136 resources)
 + raw 1962157 bytes
 + gzip 925154 bytes
 + brotli 745385 bytes
> ads + trackers (102385 network + 0 hide + 0 resources)
 + raw 3110041 bytes
 + gzip 1744912 bytes
 + brotli 1459725 bytes
> ads + trackers (102385 network + 0 hide + 136 resources)
 + raw 3223773 bytes
 + gzip 1780841 bytes
 + brotli 1491153 bytes
> ads + trackers + annoyances (104565 network + 69090 hide + 0 resources)
 + raw 6637173 bytes
 + gzip 3468287 bytes
 + brotli 2859874 bytes
> ads + trackers + annoyances (104565 network + 69090 hide + 136 resources)
 + raw 6750905 bytes
 + gzip 3502073 bytes
 + brotli 2888764 bytes
> ads + trackers + annoyances (0 network + 69090 hide + 0 resources)
 + raw 3457701 bytes
 + gzip 1673521 bytes
 + brotli 1361536 bytes
> ads + trackers + annoyances (0 network + 69090 hide + 136 resources)
 + raw 3571433 bytes
 + gzip 1707789 bytes
 + brotli 1391735 bytes
> ads + trackers + annoyances (104565 network + 0 hide + 0 resources)
 + raw 3179781 bytes
 + gzip 1793331 bytes
 + brotli 1502017 bytes
> ads + trackers + annoyances (104565 network + 0 hide + 136 resources)
 + raw 3293513 bytes
 + gzip 1829042 bytes
 + brotli 1532153 bytes

@seia-soto seia-soto requested a review from chrmod December 11, 2024 07:45
remusao
remusao reviewed Dec 25, 2024
View reviewed changes
packages/adblocker/src/resources.ts
@@ -389,17 +398,14 @@ export default class Resources {
}

estimatedSize += 2 * sizeOfByte();
estimatedSize += this.scriptlets.length; // mask
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't multiply the single byte size which equals to 1. I recall that I got a feedback regarding this before. I'll multiply by the size of single byte to make the code verbose. Sorry.

remusao
remusao reviewed Dec 25, 2024
View reviewed changes
packages/adblocker/src/resources.ts
Comment on lines -421 to +427
for (const alias of aliases) {
buffer.pushASCII(alias);
}
aliases.forEach((alias) => buffer.pushASCII(alias));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit. I don't see this necessarily as an improvement given we use for .. of in other places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@remusao remusao remusao left review comments

@chrmod chrmod Awaiting requested review from chrmod

Assignees

@seia-soto seia-soto

Labels
PR: Internal 🏠 Changes only affect internals
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@seia-soto @remusao