Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove unneeded base64 encoders #128

Merged
merged 2 commits into from
Oct 14, 2019

Conversation

thboop
Copy link
Collaborator

@thboop thboop commented Oct 11, 2019

Context
Server PR with test!
Essentially, these extra encoders weren't really doing anything besides slowing down compute, so we are safe to remove them.

@thboop thboop requested a review from TingluoHuang October 11, 2019 15:26
// 000000|00 0000|0000 00|000000| 000000|00 0000|0000 00|000000|
// Char1 Char2 Char3 Char4
// See the above, the first byte has a character beginning at index 0, the second byte has a character beginning at index 4, the third byte has a character beginning at index 2 and then the pattern repeats
// We register byte offsets for all these possible values
public static String Base64StringEscapeShift1(String value)
Copy link
Member

Choose a reason for hiding this comment

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

should we keep 0/2/4? not 0/1/2

Copy link
Member

Choose a reason for hiding this comment

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

Right, I thought it was the even bit shifts ...

Copy link
Member

Choose a reason for hiding this comment

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

it should be 0/1/2, since it's byte... not bit...
the shift is for shifting bytes.

Copy link
Member

Choose a reason for hiding this comment

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

I added an L0 for this: #129

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a Server side pr with tests as well!

The ticket has information on why these shifts were selected!

@TingluoHuang TingluoHuang merged commit c3b11b3 into master Oct 14, 2019
@TingluoHuang TingluoHuang deleted the users/thboop/removeUnneededMatchers branch October 14, 2019 14:47
AdamOlech pushed a commit to antmicro/runner that referenced this pull request Jan 28, 2021
* Remove unneeded base64 encoders

* update comment
volker-raschek pushed a commit to dedalus-cis4u/github-runner-role that referenced this pull request Jul 25, 2022
Allow runner_org to be provided as string.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants