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

WIP: Speedup #1734

Closed
wants to merge 41 commits into from
Closed

WIP: Speedup #1734

wants to merge 41 commits into from

Conversation

lfdversluis
Copy link

DO NOT MERGE, WIP!

Just making sure I do not break stuff while speeding up things.
I added tons of TODO's of items that are unclear to me, if you know their meaning, please leave a comment!

@tribler-ci
Copy link
Contributor

Can one of the admins verify this patch?

@lfdversluis
Copy link
Author

@whirm Can you run jenkins on this stuff to ensure I do not break stuff?

@whirm
Copy link

whirm commented Nov 19, 2015

ok to test

@tribler-ci
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org//job/GH_Tribler_pull-request-tester_devel/5103/
Test FAILed.

@@ -43,13 +43,17 @@ def verify_and_generate_shared_secret(self, dh_secret, dh_received, auth, B):
return shared_secret

def generate_session_keys(self, shared_secret):
# TODO hkdf???

Choose a reason for hiding this comment

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

Not the author, but Key Derivation Function - Wiki

Copy link
Author

Choose a reason for hiding this comment

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

@Pathemeous I read the wiki yesterday but didn't comment yet, thanks for the link!

Choose a reason for hiding this comment

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

Btw, this was all implemented after this issue
#1066

@whirm
Copy link

whirm commented Nov 19, 2015

@lfdversluis Don't make too big PR's, better to get incremental improvements than a huge patchset with many different things piled together.

@lfdversluis
Copy link
Author

@whirm I am fixing typos and stuff when I encounter them. The big changes in spaces are because I reformatted the code with pycharm. Are that the additional improvements you are referring to?

hkdf = HKDFExpand(algorithm=hashes.SHA256(), backend=default_backend(), length=40, info="key_generation")
key = hkdf.derive(shared_secret)

# TODO what is this

Choose a reason for hiding this comment

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

We've asked the HKDF for a string of length 40. This gives us:

  • two 128 bit keys
  • two 32 bit salts

@tribler-ci
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org//job/GH_Tribler_pull-request-tester_devel/5121/
Test FAILed.

@tribler-ci
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org//job/GH_Tribler_pull-request-tester_devel/5122/
Test FAILed.

@whirm
Copy link

whirm commented Nov 20, 2015

@lfdversluis I guess you don't know about git revlog? :)

I'll give you guys a quick git intro next week if you want.

@lfdversluis
Copy link
Author

Well since I made changes to the submodule I got a detached head, which you normally can abandon by resetting. But somehow that didn't work out and git decided that a merge was needed (?) and sort of undid my last few commits...

But I have not heard about revlog, so that might be nice to know if it solves issues such as these 👍

@tribler-ci
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org//job/GH_Tribler_pull-request-tester_devel/5123/
Test FAILed.

@tribler-ci
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org//job/GH_Tribler_pull-request-tester_devel/5124/
Test FAILed.

@lfdversluis
Copy link
Author

@whirm The tests that fail, are those "discover" tests that might fail occasionally or did I break something?

@whirm
Copy link

whirm commented Nov 24, 2015

That's why I said it would be better to first fix the community and its tests before starting optimizing it. It's buggy and some tests are broken and not always pass.

I would go for fixes until it passes 100% of the time. Then optimizations.

@lfdversluis
Copy link
Author

@whirm I don't recall you said that, but that may be a good idea. Maybe that will also help me understanding the inner workings and structure of the tunnels. Concerning GDB, I have no experience at all with GDB nor C/CPP so at most I can look in which lib it goes wrong, get a stacktrace and report it upstream to that library.

@whirm
Copy link

whirm commented Nov 24, 2015

It's probably our code doing the wrong thing. I wouldn't report anything until we are sure it's not a bug on our side.

gdb has python compatibility so you can inspect both the python and the native side.

I don't know if you should be doing that or not (@synctext ?) but it's something worth learning :)

@whirm whirm added the WIP label Dec 28, 2015
@lfdversluis
Copy link
Author

Closing this one because it contains old fixes which are already merged and items which are already applied. Will open a new one later.

@lfdversluis lfdversluis closed this Jan 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants