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

Make outgoing Filetransfers round-robin. #765

Merged

iphydf merged 1 commit into TokTok:master from zoff99:zoff99/_0.1.10__parallel_fts_

Feb 17, 2018

zoff99 commented Feb 5, 2018 •

edited by iphydf

Loading

… others)

This change is

CLAassistant commented Feb 5, 2018 •

edited

Loading

All committers have signed the CLA.

zoff99 requested review from robinlinden and iphydf

February 5, 2018 20:09

zoff99 added bug messenger labels

Author

zoff99 commented Feb 5, 2018

zoff99 assigned nurupo

iphydf unassigned nurupo

SkyzohKey commented Feb 13, 2018 •

edited

Loading

@zoff99 Please sign your commits using GPG ;)
@see: https://superuser.com/questions/397149/can-you-gpg-sign-old-commits

SkyzohKey added enhancement P1 and removed bug labels

iphydf force-pushed the zoff99/_0.1.10__parallel_fts_ branch from a225aef to d10aad0 Compare

February 17, 2018 13:31

iphydf removed request for robinlinden and iphydf

February 17, 2018 13:31

iphydf changed the title ~~make outgoing Filetransfers round-robin (instead of 1 FT blocking all…~~ Make outgoing Filetransfers round-robin.

Member

iphydf commented Feb 17, 2018

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

iphydf force-pushed the zoff99/_0.1.10__parallel_fts_ branch from d10aad0 to 742a15b Compare

February 17, 2018 13:32

iphydf self-assigned this

Member

robinlinden commented Feb 17, 2018

with a few style nits. Mostly comments that don't add any value.

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 10 unresolved discussions, some commit checks pending.

toxcore/Messenger.c, line 1504 at r1 (raw file):

}

int32_t max_s32(int32_t a, int32_t b)

This can be static.

toxcore/Messenger.c, line 1509 at r1 (raw file):

}

uint16_t min_u16(uint16_t a, uint16_t b)

This can be static.

toxcore/Messenger.c, line 1536 at r1 (raw file):

    bool any_active_fts = false;

    // HINT: iterate over all possible FTs

This comment isn't really needed.

toxcore/Messenger.c, line 1540 at r1 (raw file):

        struct File_Transfers *const ft = &friendcon->file_sending[i];

        // HINT: is this an active FT?

This comment isn't really needed.

toxcore/Messenger.c, line 1545 at r1 (raw file):

            --num;

            // HINT: is FT complete?

This comment isn't really needed.

toxcore/Messenger.c, line 1557 at r1 (raw file):

            }

            // HINT: decrease free slots by the number of slots this FT uses

This comment isn't really needed.

toxcore/Messenger.c, line 1558 at r1 (raw file):

            // HINT: decrease free slots by the number of slots this FT uses
            *free_slots = max_s32(0, (int32_t) * free_slots - ft->slots_allocated);

(int32_t) * free_slots - ft->slots_allocated looks odd. Maybe (int32_t)*free_slots - ft->slots_allocated? It seems to more in line with how we do dereferencing and casts elsewhere in the code.

toxcore/Messenger.c, line 1608 at r1 (raw file):

static void do_reqchunk_filecb(Messenger *m, int32_t friendnumber, void *userdata)
{
    // HINT: no files to send

This comment isn't really needed.

toxcore/Messenger.c, line 1621 at r1 (raw file):

                                  m->friendlist[friendnumber].friendcon_id));

    // HINT: need to keep "MIN_SLOTS_FREE" slots always free

This comment isn't really needed.

toxcore/Messenger.c, line 1626 at r1 (raw file):

    bool any_active_fts = true;
    uint32_t loop_counter = 0;
    // HINT: maximum number of Filetransfer loops

This comment isn't really needed.

Comments from Reviewable

iphydf force-pushed the zoff99/_0.1.10__parallel_fts_ branch from 742a15b to 5e8f877 Compare

February 17, 2018 14:58

Member

iphydf commented Feb 17, 2018

Review status: 0 of 8 files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.

toxcore/Messenger.c, line 1504 at r1 (raw file):

Previously, robinlinden (Robin Lindén) wrote…

This can be static.

Moved to util.

toxcore/Messenger.c, line 1509 at r1 (raw file):

Previously, robinlinden (Robin Lindén) wrote…

This can be static.

Moved.

toxcore/Messenger.c, line 1536 at r1 (raw file):

Previously, robinlinden (Robin Lindén) wrote…

This comment isn't really needed.

Changed.

toxcore/Messenger.c, line 1540 at r1 (raw file):

Previously, robinlinden (Robin Lindén) wrote…

This comment isn't really needed.

Changed.

toxcore/Messenger.c, line 1545 at r1 (raw file):

Previously, robinlinden (Robin Lindén) wrote…

This comment isn't really needed.

Changed.

toxcore/Messenger.c, line 1557 at r1 (raw file):

Previously, robinlinden (Robin Lindén) wrote…

This comment isn't really needed.

Changed.

toxcore/Messenger.c, line 1558 at r1 (raw file):

Previously, robinlinden (Robin Lindén) wrote…

(int32_t) * free_slots - ft->slots_allocated looks odd. Maybe (int32_t)*free_slots - ft->slots_allocated? It seems to more in line with how we do dereferencing and casts elsewhere in the code.

astyle did this. We should move to clang-format.

toxcore/Messenger.c, line 1608 at r1 (raw file):

Previously, robinlinden (Robin Lindén) wrote…

This comment isn't really needed.

Clarified.

toxcore/Messenger.c, line 1621 at r1 (raw file):

Previously, robinlinden (Robin Lindén) wrote…

This comment isn't really needed.

Clarified.

toxcore/Messenger.c, line 1626 at r1 (raw file):

Previously, robinlinden (Robin Lindén) wrote…

This comment isn't really needed.

Clarified.

Comments from Reviewable

iphydf force-pushed the zoff99/_0.1.10__parallel_fts_ branch from b7ad7c4 to 04e8b23 Compare

February 17, 2018 16:30


          Make outgoing Filetransfers round-robin.

82662d4

Instead of 1 FT blocking all others.

iphydf force-pushed the zoff99/_0.1.10__parallel_fts_ branch from 04e8b23 to 82662d4 Compare

February 17, 2018 16:32

iphydf merged commit 82662d4 into TokTok:master

iphydf added this to the v0.2.0 milestone

This pull request was closed.

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

Labels

enhancement messenger P1