yujun777 commented on PR #69456: URL: https://github.com/apache/doris/pull/69456#issuecomment-6492497849
Correction to the memory table above, and a re-measurement. The table above varied the number of rows per group at a fixed **total row count** (10M rows), so the 8-rows-per-group case also had eight times fewer groups (1.25M instead of 10M). That is not the shape the change is about. The correct comparison keeps the **group count fixed** and grows the rows per group, i.e. 10M groups with 1 row each (10M rows) against 10M groups with 8 rows each (80M rows). Setup for the numbers below: release build, single BE restarted before every query (cold page cache on both sides), `enable_sql_cache=false`, `exec_mem_limit` raised, RSS peak delta of one query. The `MIN` runs are unaffected by this change, so the differences between their before/after values show the method's noise, roughly ±10%. **BIGINT (10M groups)** | rows per group | total rows | query | before | after | | --- | --- | --- | --- | --- | | 1 | 10M | `MIN(v)` | 956 MB | 1060 MB | | 1 | 10M | `min_topk(v, 8)` | 1851 MB | 1877 MB | | 8 | 80M | `MIN(v)` | 2044 MB | 1933 MB | | 8 | 80M | `min_topk(v, 8)` | 5183 MB | 5071 MB | **VARCHAR (10M groups)** | rows per group | total rows | query | before | after | | --- | --- | --- | --- | --- | | 1 | 10M | `MIN(v)` | 1419 MB | 1348 MB | | 1 | 10M | `min_topk(v, 8)` | 3085 MB | 3200 MB | | 8 | 80M | `MIN(v)` | 2931 MB | 2842 MB | | 8 | 80M | `min_topk(v, 8)` | 8395 MB | 8346 MB | State kept on top of `MIN`, per group: | value type | rows per group | before | after | | --- | --- | --- | --- | | BIGINT | 1 | ~90 B | ~82 B | | BIGINT | 8 | ~314 B | ~314 B | | VARCHAR | 1 | ~167 B | ~185 B | | VARCHAR | 8 | ~546 B | ~550 B | What this says: in a release build the change is at the edge of, or below, what this method resolves. At eight distinct values per group it changes nothing measurable, because the slots dominate and a slot costs the same either way (24-byte metadata plus the 8-byte value against one 32-byte slot carrying the value inline). At one value per group the state is ~24 bytes and one heap allocation smaller, which is the whole effect and is about the size of the measurement noise. The larger gain reported earlier (1959 MB to 606 MB at 10M groups) was measured on a debug/ASAN build, where every allocation carries a redzone, so the second vector per group is what dominated. The change is worth keeping for that build, and because it removes a container, but it should not be sold as a release-memory win. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
