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

Misc updates: performance, logging, hacking #142

Merged
merged 9 commits into from
Feb 10, 2022

Conversation

Nashatyrev
Copy link
Contributor

PR Description

A batch of minor updates:

Performance:

  • 3f2bba6: pass shared SecureRandom for new NodeSession instead of expensive creation a new instance
  • 91446d7: use faster non-secure Random instead of SecureRandom to generate Envelope UUID which is used internally (for logging only)
  • 6631150: use faster non-secure Random instead of SecureRandom to generate 'random message': there is no need for secure random here (even a static zeroes payload should not compromise the security)

Logging:

  • c9ce3c5: reduce logging exception noise when logging with DEBUG level

Hacking:

  • aa33963: add an option to skip ENR signature verification for testing/hacking purposes

Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

LGTM.

Comment on lines 20 to 21
Random random = ThreadLocalRandom.current();
this.id = new UUID(random.nextLong(), random.nextLong());
Copy link
Contributor

Choose a reason for hiding this comment

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

Given we only use this for logging, I wonder if we'd be better off just using AtomicLong.getAndIncrement to assign a simple unique identifier. Could seed it with a large number if we wanted to keep it reasonably long and easier to search for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, makes sense 👍 ffb9501
Minor modifications to your idea:

  • initialize the id generator with only upper 32 bits randomly generated to easily track envelope ordering with eyes.
  • return id as a hex String

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. :)

@Nashatyrev Nashatyrev merged commit 214966a into Consensys:master Feb 10, 2022
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