Improve Matrix Agg RunningStats performance by using field ordinals as hash keys

[nknize]

This improvement comes from feedback in PR #18300 where it was suggested to use ordinals as hash keys instead of field names.

[markharwood]

cc @elastic/es-search-aggs

[imotov]

@nknize is this something that we still want to do?

[nknize]

Yes. I'm marking this as a good first issue since it's not all that difficult.

[santhosh96]

Hi there! I am new to open source contribution :) Would like to work on this issue. Any context or guidance would be great!

[imrigp]

Looks like it had been already implemented here:
https://github.com/elastic/elasticsearch/commit/90b8f5d0d8ab316a88ae17a60f04aa12187cd671#diff-d8b29266c033d9d6e7cfe327be34c6e6

[nik9000]

I think this isn't done. I'd expect all these to be arrays if it were:

    /** per field sum of observations */
    protected HashMap<String, Double> fieldSum;
    /** counts */
    protected HashMap<String, Long> counts;
    /** mean values (first moment) */
    protected HashMap<String, Double> means;
    /** variance values (second moment) */
    protected HashMap<String, Double> variances;
    /** skewness values (third moment) */
    protected HashMap<String, Double> skewness;
    /** kurtosis values (fourth moment) */
    protected HashMap<String, Double> kurtosis;
    /** covariance values */
    protected HashMap<String, HashMap<String, Double>> covariances;

[SheldonWang3000]

Hi there! I'm new to elasticsearch and open source and I'd like to contribute. Could I start with this one?
And to verify the requirements, do we want to create a new HashMap<String, Integer> to store all field names and the index, then modify all maps from HashMap<String, Double> to HashMap<Integer, Double>? Or maybe modify all maps to ArrayList? And do we want the back compatibility to support reading old structures from the stream?

[ghost]

Not sure whether it will improve anything, because:

• we somehow do need to map names to their values, so there will be at least one HashMap
• the String class' hash() method under the hood caches the calculated values
• as mappings are added to all HashMaps at once, there will be no duplicate String objects

[yokeshwaran1]

Hi everyone,
I reviewed the discussions on this issue, Im interested in contributing. Has there been any recent progress by anyone? If not, I like work at improving the Matrix Agg RunningStats performance with the use of field ordinals as hash keys. I understand the concerns of @aabyodj , I like to do some performance tests to verify the improvements. Any additional context or suggestions will be helpful.

[wchaparro]

Closing as not planned, focus on ES|QL development