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

treat String as immutable in ===. part of #22193 #22954

Merged
merged 1 commit into from
Jul 26, 2017
Merged

Conversation

JeffBezanson
Copy link
Sponsor Member

This is potentially unsafe if somebody assumes that ismutable(a) && a === b implies pointer(a) == pointer(b). Hopefully in contexts where that matters you would just look at pointer(a) directly.

@JeffBezanson JeffBezanson added kind:breaking This change will break code domain:strings "Strings!" labels Jul 25, 2017
@@ -186,6 +192,13 @@ static uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v)
#else
if (v == jl_ANY_flag) return 0x8ee30bdd;
#endif
if (dt == jl_string_type) {
#ifdef _P64
return memhash_seed(jl_string_data(v), jl_string_len(v), 0xedc3b677);
Copy link
Contributor

Choose a reason for hiding this comment

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

should these be different lengths 32 vs 64?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

For whatever reason, both functions take a uint32 seed.

Copy link
Sponsor Member

@StefanKarpinski StefanKarpinski left a comment

Choose a reason for hiding this comment

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

LGTM. Surprisingly small, simple change.

@tkelman
Copy link
Contributor

tkelman commented Jul 25, 2017

@nanosoldier runbenchmarks(ALL, vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@ararslan
Copy link
Member

Nanosoldier's table reproduced with formatting fixed:

ID time ratio memory ratio
["array","convert",("Float64","Int")] 0.81 (15%) ✅ 1.00 (1%)
["array","subarray",("lucompletepivCopy!",1000)] 0.79 (15%) ✅ 1.00 (1%)
["array","subarray",("lucompletepivCopy!",250)] 0.80 (15%) ✅ 1.00 (1%)
["array","subarray",("lucompletepivCopy!",500)] 0.78 (15%) ✅ 1.00 (1%)
["array","subarray",("lucompletepivSub!",1000)] 0.80 (15%) ✅ 1.00 (1%)
["array","subarray",("lucompletepivSub!",250)] 0.85 (15%) ✅ 1.00 (1%)
["array","subarray",("lucompletepivSub!",500)] 0.80 (15%) ✅ 1.00 (1%)
["misc","splatting",(3,3,3)] 0.85 (15%) ✅ 1.00 (1%)
["problem","laplacian","laplace_iter_vec"] 0.80 (15%) ✅ 1.00 (1%)
["problem","stockcorr","stockcorr"] 0.83 (15%) ✅ 1.00 (1%)
["scalar","mod2pi",("argument reduction (easy) abs(x) < 2.0^20π/4","positive argument","Float64")] 0.82 (15%) ✅ 1.00 (1%)
["scalar","mod2pi",("argument reduction (easy) abs(x) < 9π/4","negative argument","Float64")] 0.84 (15%) ✅ 1.00 (1%)
["scalar","mod2pi",("argument reduction (easy) abs(x) > 2.0^20*π/2","negative argument","Float64")] 1.24 (15%) ❌ 1.00 (1%)
["scalar","mod2pi",("argument reduction (easy) abs(x) > 2.0^20*π/2","positive argument","Float64")] 1.28 (15%) ❌ 1.00 (1%)
["scalar","rem_pio2",("argument reduction (easy) abs(x) < 2π/4","positive argument","Float64")] 1.18 (15%) ❌ 1.00 (1%)
["scalar","rem_pio2",("argument reduction (easy) abs(x) < 9π/4","negative argument","Float64")] 0.85 (15%) ✅ 1.00 (1%)
["scalar","rem_pio2",("argument reduction (easy) abs(x) < 9π/4","positive argument","Float64")] 1.18 (15%) ❌ 1.00 (1%)
["scalar","rem_pio2",("argument reduction (hard) abs(x) < 2π/4","negative argument","Float64")] 1.16 (15%) ❌ 1.00 (1%)
["scalar","rem_pio2",("argument reduction (paynehanek) abs(x) > 2.0^20*π/2","negative argument","Float64")] 1.23 (15%) ❌ 1.00 (1%)
["scalar","rem_pio2",("argument reduction (paynehanek) abs(x) > 2.0^20*π/2","positive argument","Float64")] 1.21 (15%) ❌ 1.00 (1%)
["tuple","reduction",("sum",(16,16))] 0.85 (15%) ✅ 1.00 (1%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:strings "Strings!" kind:breaking This change will break code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants