Opened 7 months ago

Closed 5 months ago

#21663 closed task (implemented)

prop278: Refactor the torgzip module to support additional compression schemes

Reported by: ahf Owned by: ahf
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: TorCoreTeam201703, prop278
Cc: Actual Points:
Parent ID: Points: 3
Reviewer: Sponsor: Sponsor4


The current torgzip module should be refactored such that the new compression schemes needed for prop#278 can fit nicely into the code.

This is the tracking bug for this task.

Child Tickets

#21664closedahfprop278: Make the current 'torgzip' module a submodule of a new 'compression' moduleCore Tor/Tor

Change History (12)

comment:1 Changed 7 months ago by ahf

Owner: set to ahf
Status: newaccepted

comment:2 Changed 7 months ago by ahf

Actual Points: 3
Points: 3

comment:3 Changed 7 months ago by ahf

Keywords: TorCoreTeam201703 added

comment:4 Changed 6 months ago by ahf

Keywords: prop278 added

Add prop278 keyword.

comment:5 Changed 5 months ago by ahf

Starting to make my big zstd/xz branch reviewable in

Still a few things missing before I mark this as needs_review, but this will make it easier for other people to track the progress here.

comment:6 Changed 5 months ago by nickm

Awesome! Is the list of missing features recorded somewhere? That will help distinguish "absent but planned" from "absent and not planned"

comment:7 Changed 5 months ago by ahf

The patches in my prop278/21663 branch is the cleaned up versions of the patches in bugs/21663 branch which is currently a bit of a mess.

The only part missing in the bugs/21663 branch is currently the streaming code for Zstd/LZMA, but the tor_compress() and tor_uncompress() functionality is working (and passing tests).

I think it's realistic that this bug will be marked needs_review tonight (Danish time) when I'm done splitting things up into something that is reviewable, which will include patches for #21664 and #21662.

#21665 should be a very short fix, but is not included in the bugs/21663 branch currently; we currently pass UINT64_MAX to disable the upper bound of memory consumption for LZMA.

Last edited 5 months ago by ahf (previous) (diff)

comment:8 Changed 5 months ago by nickm

I looked it over, and what's there looked fine, but I didn't see any actual lzma2 or zstd support. That's coming in the streaming code? If so I'll have another look later in the day

comment:9 Changed 5 months ago by ahf

Status: acceptedneeds_review

Refactoring and clean-up code for this issue and issue #21664 can be found in:

comment:10 Changed 5 months ago by ahf

The Zstd/LZMA code is in the big messy branch (bugs/21663) which I'm turning into smaller reviewable patches in the respective issues here on trac.

comment:11 Changed 5 months ago by nickm

Status: needs_reviewneeds_revision

Looks pretty good to me! The only issues I noted on the ticket were pretty minor.

comment:12 Changed 5 months ago by nickm

Resolution: implemented
Status: needs_revisionclosed


Note: See TracTickets for help on using tickets.