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

Port changes to encoding/hex #1

Closed
dhui opened this issue Oct 6, 2017 · 12 comments
Closed

Port changes to encoding/hex #1

dhui opened this issue Oct 6, 2017 · 12 comments

Comments

@dhui
Copy link

dhui commented Oct 6, 2017

Any plans to port these changes to the stdlib? e.g. encoding/hex
The performance improvements look awesome!

@tmthrgd
Copy link
Owner

tmthrgd commented Nov 21, 2017

At this stage there aren't and I'm not sure how possible that would be. It could be possible but it's likely the assembly would have to be rewritten to avoid licensing issues.

Out of interest, do you have a specific need for this sort of performance? This project was more a toy project for me and I've never quite found the right use for it.

@dhui
Copy link
Author

dhui commented Nov 21, 2017

I don't have a specific need. Just thought that the rest of the community could benefit from the improved performance!

@pascaldekloe
Copy link

@WojciechMula, if you aggree together with @tmthrgd to transfer your copyrights to the “Go authors” then I'll be happy to make a change request and get this puppy in. Think of all the electricity we could save worldwide. :-)

@tmthrgd
Copy link
Owner

tmthrgd commented Apr 29, 2018

@pascaldekloe If the go authors are willing to accept an assembly hex implementation, I’m game. I am a little doubtful of that, but I could well be wrong. I’d be happy to help make it happen.

As for licensing, I’m happy to sign a CLA and do the legal dance.

@pascaldekloe
Copy link

I think your written permission to transfer the copyrights to the Go autors should be enough. The acceptance is a gamble indeed. You have good numbers to show for at least.

@WojciechMula
Copy link

Hi, I agree. TBH I'm really happy that @tmthrgd has applied my weird ideas and now they have a chance to be used wordwide. :)

@pascaldekloe
Copy link

https://go-review.googlesource.com/c/go/+/110195 🙏

@tmthrgd
Copy link
Owner

tmthrgd commented Apr 30, 2018

@pascaldekloe I happen to remain doubtful, but we shall see. 🙏 There are actually a few places I'd like to clean up the current assembly (mostly around runtime·support_avx being removed -- I was thinking of moving the check into go and having a seperate (en|de)codeSSE and (en|de)codeAVX, they can still JMP into a common tail routine).

@WojciechMula Where would the fun be if it wasn't for weird ideas. (I even tried to base64 but was stumped in decoding: tmthrgd/go-base64).

@pascaldekloe
Copy link

All right, we have a no-go. It was worth the try.

@tmthrgd
Copy link
Owner

tmthrgd commented May 1, 2018

@pascaldekloe That's exactly what I expected. It was worth a shot at least, there's always this package.

@tmthrgd tmthrgd closed this as completed May 1, 2018
@pascaldekloe
Copy link

@tmthrgd, you might want to get the package compatible changes in here too for interoperability.

func Decode(dst, src []byte) (int, error) {
	if len(dst) < len(src)/2 {
		panic("dst buffer is too small")
	}

	if l := uint64(len(src) &^ 1); l != 0 {
		if n, ok := decodeASM(&dst[0], &src[0], l); !ok {
			// map valid bytes
			if l := n &^ 1; l != 0 {
				decodeASM(&dst[0], &src[0], n&^1)
			}

			return int(n / 2), InvalidByteError(src[n])
		}
	}

	if len(src)&1 != 0 { // odd number of hex characters
		err := ErrLength

		// check validity last character
		if _, ok := fromHexChar(src[len(src)-1]); !ok {
			err = InvalidByteError(src[len(src)-1])
		}

		return len(src) / 2, err
	}

	return len(src) / 2, nil
}

@WojciechMula
Copy link

@tmthrgd speaking of base64, you might like to check @aklomp great library https://github.com/aklomp/base64 he used our methods, also on ARM architectures; the library's code is pleasant to read

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

No branches or pull requests

4 participants