Skip to content

Conversation

@jjlilley
Copy link

@jjlilley jjlilley commented Oct 16, 2019

Stack from ghstack:

ScriptModuleSerializer::writeCode() is the only place during torch::save()
serialization where we attempt to zip compress records.

This change avoids compressing these string records if they are
sufficiently small - e.g. in the example I looked at:

  • the strings were 123 and 28 bytes, respectively.
  • the cost in the compression routines was 16.5% of the torch::save() cost.
    (we're building a huffman table for a 28 byte string).

We'd save time and not significantly affect the space if we add these
1-line conditional compressions, rather than making it unconditional.

Differential Revision: D17967995

ScriptModuleSerializer::writeCode() is the only place during torch::save()
serialization where we attempt to zip compress records.

This change avoids compressing these string records if they are
sufficiently small - e.g. in the example I looked at:
  - the strings were 123 and 28 bytes, respectively.
  - the cost in the compression routines was 16.5% of the torch::save() cost.
    (we're building a huffman table for a 28 byte string).

We'd save time and not significantly affect the space if we add these
1-line conditional compressions, rather than making it unconditional.

Differential Revision: [D17967995](https://our.internmc.facebook.com/intern/diff/D17967995/)

[ghstack-poisoned]
@jjlilley jjlilley requested a review from apaszke as a code owner October 16, 2019 23:52
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Oct 16, 2019
jjlilley pushed a commit that referenced this pull request Oct 16, 2019
ScriptModuleSerializer::writeCode() is the only place during torch::save()
serialization where we attempt to zip compress records.

This change avoids compressing these string records if they are
sufficiently small - e.g. in the example I looked at:
  - the strings were 123 and 28 bytes, respectively.
  - the cost in the compression routines was 16.5% of the torch::save() cost.
    (we're building a huffman table for a 28 byte string).

We'd save time and not significantly affect the space if we add these
1-line conditional compressions, rather than making it unconditional.

Differential Revision: [D17967995](https://our.internmc.facebook.com/intern/diff/D17967995/)

ghstack-source-id: 92065904
Pull Request resolved: #28180
@jjlilley
Copy link
Author

fwiw, there's a pdf of the profile under D17967995...

@jjlilley jjlilley requested review from suo and zdevito October 17, 2019 00:39
…der records."

ScriptModuleSerializer::writeCode() is the only place during torch::save()
serialization where we attempt to zip compress records.

This change avoids compressing these string records if they are
sufficiently small - e.g. in the example I looked at:
  - the strings were 123 and 28 bytes, respectively.
  - the cost in the compression routines was 16.5% of the torch::save() cost.
    (we're building a huffman table for a 28 byte string).

We'd save time and not significantly affect the space if we add these
1-line conditional compressions, rather than making it unconditional.

Differential Revision: [D17967995](https://our.internmc.facebook.com/intern/diff/D17967995/)

[ghstack-poisoned]
jjlilley pushed a commit that referenced this pull request Oct 17, 2019
Pull Request resolved: #28180

ScriptModuleSerializer::writeCode() is the only place during torch::save()
serialization where we attempt to zip compress records.

This change avoids compressing these string records if they are
sufficiently small - e.g. in the example I looked at:
  - the strings were 123 and 28 bytes, respectively.
  - the cost in the compression routines was 16.5% of the torch::save() cost.
    (we're building a huffman table for a 28 byte string).

We'd save time and not significantly affect the space if we add these
1-line conditional compressions, rather than making it unconditional.
ghstack-source-id: 92104517

Differential Revision: [D17967995](https://our.internmc.facebook.com/intern/diff/D17967995/)
@jjlilley
Copy link
Author

needed to rebase, to resolve merge conflicts.

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in d7ff34c.

@facebook-github-bot facebook-github-bot deleted the gh/jjlilley/6/head branch October 28, 2019 22:16
thiagocrepaldi pushed a commit to thiagocrepaldi/pytorch that referenced this pull request Feb 4, 2020
…#28180)

Summary:
Pull Request resolved: pytorch#28180

ScriptModuleSerializer::writeCode() is the only place during torch::save()
serialization where we attempt to zip compress records.

This change avoids compressing these string records if they are
sufficiently small - e.g. in the example I looked at:
  - the strings were 123 and 28 bytes, respectively.
  - the cost in the compression routines was 16.5% of the torch::save() cost.
    (we're building a huffman table for a 28 byte string).

We'd save time and not significantly affect the space if we add these
1-line conditional compressions, rather than making it unconditional.
ghstack-source-id: 92104517

Test Plan:
Benchmark: experimental/jeremyl/c2:SerializationBench
  Correctness: normal buck mode/dev-nosan caffe2/test/...

Differential Revision: D17967995

fbshipit-source-id: 7ff934388533645dc987e105c814ffe6324f4596
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants