improve accuracy of ambiguity checking (take 2)

[vtjnash]

There was a small typo in the original PR that caused this to disable a useful fast path in a few cases. Users were also unhappy to discover that poor type-intersection results that steal away performance were now being reported. Hide those again to calm the irate masses. Also finishes fixing detect_unbound_args and detect_ambiguities to correctly support #31916 (which only implemented a subset of the required corrections). This should now be faster and simpler to use, as it can avoid substantial excess work when used by a package to check itself.

[KristofferC]

And I guess #37458 has been fixed not to happen again?

[vtjnash]

Yes that’s the expectation, as I mentioned above, this continues to explicitly skip reporting ambiguities that might occur when type intersection Information seems poor. It remains a probable performance drain, but we’ll continue to avoid reporting them.

[KristofferC]

For me this still seems to slow down Plots.jl time:

Before

julia> @time using Plots
  2.771853 seconds (6.19 M allocations: 442.641 MiB, 3.36% gc time)

After

julia> @time using Plots
  3.606244 seconds (7.18 M allocations: 497.321 MiB, 3.48% gc time)

Is that just for me?

[vtjnash]

That's way below the variability I see for just building master. It can be somewhat more accurately measured by only rebuilding the src files. There's a small additional cost (I'd say about 10%), but that is just reflective of the bad job type-intersection is already doing on a range of important queries and so should see a more substantial speedup from fixing that (#36951).

[KristofferC]

It seems pretty consistent to me:

PR:

~/julia master* ⇡
❯ ./julia -e '@time using Plots'
  3.528268 seconds (7.18 M allocations: 497.487 MiB, 2.88% gc time)

~/julia master* ⇡
❯ ./julia -e '@time using Plots'
  3.538384 seconds (7.18 M allocations: 497.479 MiB, 2.99% gc time)

~/julia master* ⇡
❯ ./julia -e '@time using Plots'
  3.539718 seconds (7.18 M allocations: 497.496 MiB, 3.17% gc time)

Master:

  ~/julia master*
❯ ./julia -e '@time using Plots'
  3.044624 seconds (6.15 M allocations: 441.648 MiB, 4.69% gc time)

~/julia master*
❯ ./julia -e '@time using Plots'
  2.919994 seconds (6.15 M allocations: 441.648 MiB, 4.92% gc time)

~/julia master*
❯ ./julia -e '@time using Plots'
  3.000036 seconds (6.15 M allocations: 441.650 MiB, 4.53% gc time)

A blanket 17% regression seems significant. Are you saying there is other work in the pipeline that brings this back or that we just have to eat this regression? Just looking at it from an outsider's p.o.v it seems unfortunate but I realize I don't have all the context for what this change does and what's upcoming.

[vtjnash]

I can't tell exactly what you're measuring just from that, but it looks like you're not actually rebuilding, so the repeated trials aren't providing any useful additional information

[vtjnash]

Are you saying there is other work in the pipeline

I don't know if the work is in the pipeline, but that is why I keep pointing to the issue issue I created to document why this is an existing problem with master #36951

A blanket 17% regression seems significant

Such a number is absolutely devoid of meaning without context. The context I'm attempting to give is that #36951 appears to cause significant extra work (disabling just some of the effect of it seems to take the ambiguity test here from about 80 seconds to about 14). On balance, this is intended to help stabilize #36733 to ensure we don't get odd ordering-related dependencies where latency times suddenly spike in certain circumstances (rebuilds).