PR#6217 Improve perf. of functional record update #538
Conversation
| @@ -1235,7 +1235,8 @@ and transl_setinstvar self var expr = | |||
| and transl_record env all_labels repres lbl_expr_list opt_init_expr = | |||
| let size = Array.length all_labels in | |||
| (* Determine if there are "enough" new fields *) | |||
| if 3 + 2 * List.length lbl_expr_list >= size | |||
| let no_init = match opt_init_expr with None -> true | _ -> false in | |||
| if no_init || size > 255 | |||
There was a problem hiding this comment.
Should this be Config.max_young_wosize rather than "255"?
There was a problem hiding this comment.
Indeed, thank you.
|
I'd be interested in seeing the benchmarks, if that's possible. Perhaps just attach the benchmark details to the PR? |
|
Hello, |
|
Thanks for the detailed measurements. I merged the pull request in trunk. |
| (* Determine if there are "enough" fields (only relevant if this is a | ||
| functional-style record update *) | ||
| let no_init = match opt_init_expr with None -> true | _ -> false in | ||
| if no_init || size >= Config.max_young_wosize |
There was a problem hiding this comment.
The direction of this test is wrong: should be size <= Config.max_young_wosize
There was a problem hiding this comment.
Oops… sorry about that. Should I make a new pull request?
I believe "<" would be better since the caml_modify method is already faster with a 256 fields.
There was a problem hiding this comment.
this will cause code bloat in pathological cases
There was a problem hiding this comment.
What cases do you have in mind? If the size is of the record is larger than 256 then it uses the "duplicate and modify" approach which requires less code.
PR#6217 Improve perf. of functional record update
Co-authored-by: Thibaut Mattio <thibaut.mattio@gmail.com>
See Mantis issue #6217. Benchmarking with
Core_benchon a 2.5 GHz 64-bit Intel i5-2450M showed that fieldwise copy is always faster while the number of fields is smaller thanMax_young_wosize(that is, 256); beyond that point the "duplication and modification" method becomes much faster.Benchmark details:
rfu_benchmarks.pdf
CSV results:
rfu_bench.zip