Skip to content

Use ResourceHash in blob data provider#1574

Merged
Manishearth merged 6 commits intounicode-org:mainfrom
Manishearth:resourcehash-ule
Feb 2, 2022
Merged

Use ResourceHash in blob data provider#1574
Manishearth merged 6 commits intounicode-org:mainfrom
Manishearth:resourcehash-ule

Conversation

@Manishearth
Copy link
Member

Encountered a minor VarZeroVec bug where the debug integrity check did not handle inserting to vectors only containing empty elements. This was fixed by changing the integrity check and adding a test. I think we didn't hit this previously due to pure chance: by switching to ResourceKeyHash the vector got reordered and it happened to start with an empty push.

cc @robertbastian

@Manishearth Manishearth requested review from a team and sffc as code owners February 2, 2022 19:55
robertbastian
robertbastian previously approved these changes Feb 2, 2022
return false;
}
if slice_len <= 4 + len as usize * 4 {
if slice_len < 4 + len as usize * 4 {
Copy link
Member

Choose a reason for hiding this comment

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

I pretty much had this code yesterday but gave up when I hit the validation error.

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • provider/testdata/data/testdata.postcard is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@sffc
Copy link
Member

sffc commented Feb 2, 2022

Code size info:

Before #1569:

-rwxr-xr-x 1 runner docker    41680 Feb  2 06:51 optim4.elf
-rwxr-xr-x 1 runner docker    31768 Feb  2 06:51 optim5.elf

After #1569:

-rwxr-xr-x 1 runner docker    42512 Feb  2 08:25 optim4.elf
-rwxr-xr-x 1 runner docker    32664 Feb  2 08:25 optim5.elf

After this PR:

-rwxr-xr-x 1 runner docker    41360 Feb  2 23:16 optim4.elf
-rwxr-xr-x 1 runner docker    31592 Feb  2 23:16 optim5.elf

This is great: ZeroMap2d added a little bit of code size, but the change to ResourceKeyHash more than compensates for that, bringing overall code size down from what it was before.

@Manishearth Manishearth merged commit 10f3d2d into unicode-org:main Feb 2, 2022
@Manishearth Manishearth deleted the resourcehash-ule branch February 2, 2022 23:50
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.

3 participants