[SPARK-21603][SQL][FOLLOW-UP] Change the default value of maxLinesPerFunction into 4000

[maropu]

What changes were proposed in this pull request?

This pr changed the default value of maxLinesPerFunction into 4000. In #18810, we had this new option to disable code generation for too long functions and I found this option only affected Q17 and Q66 in TPC-DS. But, Q66 had some performance regression:

Q17 w/o #18810, 3224ms --> q17 w/#18810, 2627ms (improvement)
Q66 w/o #18810, 1712ms --> q66 w/#18810, 3032ms (regression)

To keep the previous performance in TPC-DS, we better set higher value at maxLinesPerFunction by default.

How was this patch tested?

Existing tests.

[maropu]

I'll check performance changes in TPC-DS.

[maropu]

Got the numbers and see them here. The numbers seems to be the almost same with the master just before #18810 merged. @gatorsmile@viirya

[maropu]

Also, I talked with @viiryahere and -1 seems to be more natural to disable this too-long-function option. If we don't strongly disagree with this, I could make another trivial pr to fix, too. @gatorsmile

[SparkQA]

Test build #80976 has finished for PR 19021 at commit d0bc6a6.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[gatorsmile]

Could you also try 2731 and 2049? and see the perf difference?

8K = 8192 8192 / 3 + 1 = 2731
8192 / 2 + 1 = 4097

[maropu]

I've already checked 2800; this value activated too-long-function optimization in Q17/Q66 and Q66 had regression. So, I think 2731 and 2049 possibly have the same regression (I've also checked 2900 that disabled this in Q17/Q66).

btw, why the computation is based on 8K? It seems the threshold of DontCompileHugeMethods is 8000?

[gatorsmile]

YES.... Just double check it. The default of HugeMethodLimitis 8000... Thanks!

[gatorsmile]

LGTM

[gatorsmile]

cc @rednaxelafx

[gatorsmile]

Thanks! Merging to master.

[maropu]

Just for your info, again, I looked into this issue in TPC-DS quries; I added some code to check the actual bytecode size of these quries and I found the gen'd function in Q17/Q66 only had too-long bytecode over 8000:

===== TPCDS QUERY BENCHMARK OUTPUT FOR q17 =====
17/08/23 14:45:02 WARN CodeGenerator: GeneratedClass.agg_doAggregateWithKeys is too large to do JIT compilation on HotSpot; the size of agg_doAggregateWithKeys is 17665; the limit is 8000
===== TPCDS QUERY BENCHMARK OUTPUT FOR q66 =====
17/08/23 14:55:39 WARN CodeGenerator: GeneratedClass.agg_doAggregateWithKeys is too large to do JIT compilation on HotSpot; the size of agg_doAggregateWithKeys is 11012; the limit is 8000
17/08/23 14:55:39 WARN CodeGenerator: GeneratedClass.agg_doAggregateWithKeys is too large to do JIT compilation on HotSpot; the size of agg_doAggregateWithKeys is 13420; the limit is 8000
17/08/23 14:55:39 WARN CodeGenerator: GeneratedClass.agg_doAggregateWithKeys is too large to do JIT compilation on HotSpot; the size of agg_doAggregateWithKeys is 16641; the limit is 8000

BTW, why we don't check if gen'd bytecode size is over 8000 directly instead of code line num. in #18810? cc: @gatorsmile@viirya@kiszk

[viirya]

@maropu Should be a good idea. Especially the number of lines of code may not be intuitive to set for this purpose.