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

Refactor UnmarshalText in order to parse hash-like UUID. #54

Closed
wants to merge 5 commits into from
Closed

Refactor UnmarshalText in order to parse hash-like UUID. #54

wants to merge 5 commits into from

Conversation

daskol
Copy link
Contributor

@daskol daskol commented Aug 8, 2017

I refactor UnmarshalText in order to support yet another text representation of UUID lilke 6ba7b8109dad11d180b400c04fd430c8. The reason is that this format is quite frequent.

I tried to introduce BNF of what UmarshalText actually does. In fact it simplifies implementation and future support. On other hand current implementation led to performance degradation(see below before/after).

BEFORE

BenchmarkFromString-4               	10000000	       122 ns/op
BenchmarkFromStringUrn-4            	10000000	       124 ns/op
BenchmarkFromStringWithBrackets-4   	10000000	       123 ns/op
BenchmarkUnmarshalText-4            	20000000	        81.6 ns/op
PASS
ok  	github.com/daskol/go.uuid	25.786s

AFTER

BenchmarkFromString-4               	20000000	        91.7 ns/op
BenchmarkFromStringUrn-4            	 3000000	       493 ns/op
BenchmarkFromStringWithBrackets-4   	 5000000	       358 ns/op
BenchmarkUnmarshalText-4            	 2000000	       612 ns/op
PASS
ok  	github.com/daskol/go.uuid	27.822s

Eventually correct UnmarshalText should be based on FSA to avoid overhead during parsing.

@coveralls
Copy link

coveralls commented Aug 8, 2017

Coverage Status

Coverage decreased (-1.9%) to 94.702% when pulling 6902e4b on daskol:parse-from-string into 5bf94b6 on satori:master.

@daskol
Copy link
Contributor Author

daskol commented Aug 8, 2017

It is wondered that simple heuristics improves perfomance significantly.

NOW

BenchmarkFromString-4               	20000000	        92.3 ns/op
BenchmarkFromStringUrn-4            	20000000	       102 ns/op
BenchmarkFromStringWithBrackets-4   	20000000	       106 ns/op
BenchmarkUnmarshalText-4            	10000000	       156 ns/op
PASS
ok  	github.com/daskol/go.uuid	28.302s

@coveralls
Copy link

coveralls commented Aug 8, 2017

Coverage Status

Coverage decreased (-5.7%) to 90.82% when pulling 23c4681 on daskol:parse-from-string into 5bf94b6 on satori:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-5.7%) to 90.82% when pulling 23c4681 on daskol:parse-from-string into 5bf94b6 on satori:master.

@daskol
Copy link
Contributor Author

daskol commented Oct 31, 2017

@satori Your resolution needed!

satori added a commit that referenced this pull request Jan 2, 2018
Refactor UnmarshalText in order to parse hash-like UUID.
@satori
Copy link
Owner

satori commented Jan 2, 2018

@daskol I refactored decodeCanonical to remove extra memory allocations which caused performance degradation. That and few other style fixes were merged into 8be4f1c

Thanks for your contribution.

@satori satori closed this Jan 2, 2018
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.

3 participants