[
https://issues.apache.org/jira/browse/SPARK-58677?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Nathan Holland updated SPARK-58677:
-----------------------------------
Description:
This ticket expands on SPARK-45900, to explicitly mention the desire for a
BINARY return type for xxhash3_128 as well as additional functionality that a
modern hashing algorithm on Spark could implement. (such as deterministic
support for complex types)
The original intent of that ticket was to capture a new hashing model through
XXH3 that allowed for high performance hashing using XXH3 with the API of the
earlier XXHash64 hash family, rather than some of the cryptographic hash
functions (SHA/MD5 etc)
h2. Return Type
One difference in this model I am proposing - SPARK-45900 has been resolved by
returning a string, which is more consistent with the older hashing models.
When writing SPARK-45900 I mentioned that this was a downside of the existing
SHA models, to clarify returning the 128-bit result as 16-byte binary avoids
hexadecimal conversion and uses half the storage of a hex result. Its fixed
result width also gives JVM and native vectorized execution engines (like
Photon) opportunities to optimize equality comparisons using fixed-width SIMD
operations. Furthermore I've been experimenting with expanding the BINARY type
to a "FIXEDBINARY" type and have seen some promising performance increases,
which might tie into this nicely.
I believe now would be the best time to discuss what this API should return
before we get to the point that 4.4 gets a public API free - and either choose
with a seperate API that returns the BINARY type or make the change to
SPARK-45900 to return a BINARY type by default and support a xxh3_128_hex
method for those who prefer STRING as a return type.
You can see a few instances where return bytes[]/unsigned 128 ints or hex
specific variants below, so I believe there is presidence for this decision:
* [https://duckdb.org/community_extensions/extensions/hashfuncs] (duckdb
communit hashfuncs sporting *xxh3_128* and {*}xxh3_128_hex{*}) *- I think this
is the best method to allow for a hex code when needed, while still allowing a
byte[] for performance and compact storage when needed.*
*
[https://clickhouse.com/docs/reference/functions/regular-functions/hash-functions#xxh3_128]
(Clickhouse definition 128 unsigned int which without native support is
approximately equivalent to a byte[16])
*
[https://learn.microsoft.com/en-us/dotnet/api/system.io.hashing.xxhash128.hash?view=net-11.0-pp#system-io-hashing-xxhash128-hash(system-byte())]
(C# implementation returning byte[16])
* [https://github.com/Cyan4973/xxHash/blob/dev/xxhash.h#L1378] (the reference
using two long's within a struct, technically we could return a struct with a
high and a low long like this, but I would worry about allocations until
Valhalla drops and I feel like byte[16] is probably a better fit anyway)
To summarise this would create a simplified function base like the below,
allowing for some good performance uplift with xxh3_128 in future through
Vectorisation, while xxh3_128_hex is still there for people who prefer human
readable values / want a drop in replacement for the likes of MD5 without
wrapping it in a hex() string.
* xxh3_64(...) -> BIGINT
* xxh3_128(...) -> 16-byte BINARY
* xxh3_128_hex(...) -> 32-character lowercase STRING{{{{}}{}}}
h2. XXHash64 Style Hash API
One of the benefits that made me open the original ticket is the feedback from
customers that one of xxhash64's benefits from an API perspective is the simple
API to support multiple columns which conceptually allow the hashing of columns
by passing them in natively without having to implement brittle solutions
(often something like concat_ws or to_json) to hash data that spans columns. I
believe an expansion to the functionality to SPARK-45900 to support columns
would allow XXH3 to be seen successor to the XXHash64 function with this user
friendly API - while keeping all the semantics that a unitary function would
support - including byte compatibility with the original C reference code.
_Note: An earlier version of this ticket had a reference to some structural
changes to the way hashing works in Spark - but I've since decided that
changing hashing semantics is probably too broad a scope change for now and
I'm' winding that back into a seperate proposal._
h2. Proposal Codebase
My current codebase can be found here for a proposed implementation of the
above two key changes
* [https://github.com/NathanNZ/spark/tree/SPARK-58677-xxh3]
* I've also got a work in progress solution with pre-built containers to test
the functionality.
** [https://github.com/NathanNZ/spark-nathan-containers]
Links:
- https://issues.apache.org/jira/browse/SPARK-45900 (Original SPARK-45900
ticket)
was:
This ticket expands on SPARK-45900, to explicitly mention the desire for a
BINARY return type for xxhash3_128 as well as additional functionality that a
modern hashing algorithm on Spark could implement. (such as deterministic
support for complex types)
The original intent of that ticket was to capture a new hashing model through
XXH3 that allowed for high performance hashing using XXH3 with the API of the
earlier XXHash64 hash family, rather than some of the cryptographic hash
functions (SHA/MD5 etc)
h2. Return Type
One difference in this model I am proposing - SPARK-45900 has been resolved by
returning a string, which is more consistent with the older hashing models.
When writing SPARK-45900 I mentioned that this was a downside of the existing
SHA models, to clarifify - returning a STRING type has heavier overhead and
prevents future optimisations that occur by allowing HotSpot (and C
interpreters like Photon) to take advantage of SIMD instructions. Furthermore
I've been experimenting with expanding the BINARY type to have fixed byte types
and have seen some promising performance increases that may be useful at a
later date.
As XXH3 was built specifically around SIMD instructions to provide speed ups
over XXHash64 and Spark 4.4 is not out yet, I believe now would be the time to
discuss what this API should return - and either choose with a seperate API
that returns the BINARY 128 bytes or make the change to the solution of
SPARK-45900 to return a BINARY type by default and let users use the "hex"
function if they want the result as a Binary String. I'd propose that if we
preserve the hex functionality we suffix it as _hex, which allows us to split
up the raw bytes and the friendlier strings if required.
You can see a few instances that return bytes[]/unsigned 128 ints or hex
specific variants below, so I believe there is presidence
* [https://duckdb.org/community_extensions/extensions/hashfuncs] (duckdb
communit hashfuncs sporting *xxh3_128* and {*}xxh3_128_hex{*}) *- I think this
is the best method to allow for a hex code when needed, while still allowing a
byte[] for performance and compact storage when needed.*
*
[https://clickhouse.com/docs/reference/functions/regular-functions/hash-functions#xxh3_128]
(Clickhouse definition 128 unsigned int which without native support is
approximately equivalent to a byte[128])
*
[https://learn.microsoft.com/en-us/dotnet/api/system.io.hashing.xxhash128.hash?view=net-11.0-pp#system-io-hashing-xxhash128-hash(system-byte())]
(C# implementation returning byte[128])
* [https://github.com/Cyan4973/xxHash/blob/dev/xxhash.h#L1378] (the reference
using two long's within a struct, technically we could return a struct with a
high and a low long like this, but I would worry about allocations until
Valhalla drops and I feel like byte[128] is probably a better fit anyway)
To summarise this would create a simplified function base like the below,
allowing for some good performance uplift with xxh3_128 in future through
Vectorisation, while xxh3_128_hex is still there for people who prefer human
readable values / want a drop in replacement for the likes of MD5 without
wrapping it in a hex() string.
* xxh3_64(...) -> BIGINT
* xxh3_128(...) -> 16-byte BINARY
* xxh3_128_hex(...) -> 32-character lowercase STRING{{{{}}{}}}
h2. XXHash64 Style Hash API
One of the benefits that made me open the original ticket is the feedback from
customers that one of xxhash64's benefits from an API perspective is the simple
API to support multiple columns which conceptually allow the hashing of columns
by passing them in natively without having to implement brittle solutions
(often something like concat_ws or to_json) to hash data that spans columns. I
believe an expansion to the functionality to SPARK-45900 to support columns
would allow XXH3 to be seen successor to the XXHash64 function with this user
friendly API - while keeping all the semantics that a unitary function would
support - including byte compatibility with the original C reference code.
_Note: An earlier version of this ticket had a reference to some structural
changes to the way hashing works in Spark upon looking at the size of the
commit I'm going to move that out too another ticket. I'd assume it might even
be at the level a SPIP would make sense?_
h2. Proposal Codebase
My current codebase can be found here for a proposed implementation of the
above two key changes
* [https://github.com/NathanNZ/spark/tree/SPARK-58677-xxh3]
* I've also got a work in progress solution with pre-built containers to test
the functionality.
** [https://github.com/NathanNZ/spark-nathan-containers]
Links:
- https://issues.apache.org/jira/browse/SPARK-45900 (Original SPARK-45900
ticket)
> Expand hash functionalities of XXH3 to better match XXHash64
> ------------------------------------------------------------
>
> Key: SPARK-58677
> URL: https://issues.apache.org/jira/browse/SPARK-58677
> Project: Spark
> Issue Type: Improvement
> Components: PySpark, SQL
> Affects Versions: 4.4.0
> Reporter: Nathan Holland
> Priority: Major
>
> This ticket expands on SPARK-45900, to explicitly mention the desire for a
> BINARY return type for xxhash3_128 as well as additional functionality that a
> modern hashing algorithm on Spark could implement. (such as deterministic
> support for complex types)
> The original intent of that ticket was to capture a new hashing model through
> XXH3 that allowed for high performance hashing using XXH3 with the API of the
> earlier XXHash64 hash family, rather than some of the cryptographic hash
> functions (SHA/MD5 etc)
> h2. Return Type
> One difference in this model I am proposing - SPARK-45900 has been resolved
> by returning a string, which is more consistent with the older hashing
> models. When writing SPARK-45900 I mentioned that this was a downside of the
> existing SHA models, to clarify returning the 128-bit result as 16-byte
> binary avoids hexadecimal conversion and uses half the storage of a hex
> result. Its fixed result width also gives JVM and native vectorized execution
> engines (like Photon) opportunities to optimize equality comparisons using
> fixed-width SIMD operations. Furthermore I've been experimenting with
> expanding the BINARY type to a "FIXEDBINARY" type and have seen some
> promising performance increases, which might tie into this nicely.
> I believe now would be the best time to discuss what this API should return
> before we get to the point that 4.4 gets a public API free - and either
> choose with a seperate API that returns the BINARY type or make the change to
> SPARK-45900 to return a BINARY type by default and support a xxh3_128_hex
> method for those who prefer STRING as a return type.
> You can see a few instances where return bytes[]/unsigned 128 ints or hex
> specific variants below, so I believe there is presidence for this decision:
> * [https://duckdb.org/community_extensions/extensions/hashfuncs] (duckdb
> communit hashfuncs sporting *xxh3_128* and {*}xxh3_128_hex{*}) *- I think
> this is the best method to allow for a hex code when needed, while still
> allowing a byte[] for performance and compact storage when needed.*
> *
> [https://clickhouse.com/docs/reference/functions/regular-functions/hash-functions#xxh3_128]
> (Clickhouse definition 128 unsigned int which without native support is
> approximately equivalent to a byte[16])
> *
> [https://learn.microsoft.com/en-us/dotnet/api/system.io.hashing.xxhash128.hash?view=net-11.0-pp#system-io-hashing-xxhash128-hash(system-byte())]
> (C# implementation returning byte[16])
> * [https://github.com/Cyan4973/xxHash/blob/dev/xxhash.h#L1378] (the
> reference using two long's within a struct, technically we could return a
> struct with a high and a low long like this, but I would worry about
> allocations until Valhalla drops and I feel like byte[16] is probably a
> better fit anyway)
> To summarise this would create a simplified function base like the below,
> allowing for some good performance uplift with xxh3_128 in future through
> Vectorisation, while xxh3_128_hex is still there for people who prefer human
> readable values / want a drop in replacement for the likes of MD5 without
> wrapping it in a hex() string.
> * xxh3_64(...) -> BIGINT
> * xxh3_128(...) -> 16-byte BINARY
> * xxh3_128_hex(...) -> 32-character lowercase STRING{{{{}}{}}}
> h2. XXHash64 Style Hash API
> One of the benefits that made me open the original ticket is the feedback
> from customers that one of xxhash64's benefits from an API perspective is the
> simple API to support multiple columns which conceptually allow the hashing
> of columns by passing them in natively without having to implement brittle
> solutions (often something like concat_ws or to_json) to hash data that spans
> columns. I believe an expansion to the functionality to SPARK-45900 to
> support columns would allow XXH3 to be seen successor to the XXHash64
> function with this user friendly API - while keeping all the semantics that a
> unitary function would support - including byte compatibility with the
> original C reference code.
> _Note: An earlier version of this ticket had a reference to some structural
> changes to the way hashing works in Spark - but I've since decided that
> changing hashing semantics is probably too broad a scope change for now and
> I'm' winding that back into a seperate proposal._
> h2. Proposal Codebase
> My current codebase can be found here for a proposed implementation of the
> above two key changes
> * [https://github.com/NathanNZ/spark/tree/SPARK-58677-xxh3]
> * I've also got a work in progress solution with pre-built containers to
> test the functionality.
> ** [https://github.com/NathanNZ/spark-nathan-containers]
>
> Links:
> - https://issues.apache.org/jira/browse/SPARK-45900 (Original SPARK-45900
> ticket)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]