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.

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

raise the max piece size to 512 MiB and fix check when loading torrents #7736

Closed
wants to merge 1 commit into from

Conversation

arvidn
Copy link
Owner

@arvidn arvidn commented Sep 29, 2024

No description provided.

// count blocks per piece, that's restricting this
max_blocks_per_piece = (1 << 15) - 1
Copy link
Owner Author

Choose a reason for hiding this comment

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

the reason this wasn't caught early was because this 15 should have been 14. The bitfield was probably changed without this changing.


// ---- 64 bit boundary ----

// the number of dirty blocks in this piece
std::uint32_t num_dirty:14;

// the number of blocks in the cache for this piece
std::uint32_t num_blocks:14;
Copy link
Owner Author

Choose a reason for hiding this comment

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

by bumping this to 16 bits, the max piece size goes to 512 MiB

Copy link
Contributor

Choose a reason for hiding this comment

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

How about num_dirty?

Copy link
Owner Author

Choose a reason for hiding this comment

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

yeah, that should be wider too

Copy link
Contributor

Choose a reason for hiding this comment

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

How about num_dirty?

And pinned?

Copy link
Contributor

Choose a reason for hiding this comment

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

How about num_dirty?

And pinned?

I hope you haven't lost sight of it. It is also 15 bits.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm tempted to just fix the check rather than mucking about with this struct. Do you think there's a legitimate use for torrents with piece sizes this large? It really seems questionable

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think there's a legitimate use for torrents with piece sizes this large? It really seems questionable

I only know that the problem was reported by users... In addition, what seemed meaningless yesterday or today will become commonplace tomorrow.
Support for 512 MiB piece size does not require major changes, so why not add it?
Still, I don't insist. The correct processing of limits is good in itself. But I would still expand the potential capacity in upcoming major update.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think it would be much more reasonable to grow torrents in the number of pieces, rather than piece size.

@@ -97,9 +97,9 @@ namespace libtorrent {
// priority factor
prio_factor = 3,
// max blocks per piece
// there are counters in downloading_piece that only have 15 bits to
// there are counters in downloading_piece that only have 16 bits to
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't see that the counter itself was changed. Just this comment... Or is it a 16-bit size already?

Copy link
Owner Author

Choose a reason for hiding this comment

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

num_blocks went from 14 bits to a uint16_t (i.e. 16 bits)

Copy link
Contributor

Choose a reason for hiding this comment

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

num_blocks went from 14 bits to a uint16_t (i.e. 16 bits)

Ah, I thought it was about something else.

@arvidn arvidn force-pushed the max-piece-size branch 2 times, most recently from f0a1527 to 6c34b37 Compare September 29, 2024 14:30
@arvidn
Copy link
Owner Author

arvidn commented Sep 30, 2024

alternative fix: #7740

@arvidn arvidn closed this Oct 1, 2024
@arvidn arvidn deleted the max-piece-size branch October 1, 2024 22:03
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.

2 participants