QueryCompiler Batch Formula Compilation

[nbauernfeind]

The following example, which compiles 1k simple formulas, drops from 96s down to 9s on an M2 MBP:

def size = 1000
ia = new int[size]
upd = new String[size]
for (int ii = 0; ii < size; ++ii) {
    ia[ii] = 1024 * ii
    upd[ii] = "C" + ii + " = ia[" + ii + "]"
}
s = emptyTable(1).update(upd)

Fixes #4814.
Fixes #4918.
Fixes #5185.

This also addresses ambiguity when column array names clash with column names; columns trump column arrays. This query is now well defined (resulting in R = 1):

import io.deephaven.engine.util.TableTools
t_0 = TableTools.emptyTable(1).update("C = 0", "C_ = new long[] {1, 2, 3}", "R = C_[0]")
t_1 = TableTools.emptyTable(1).update("C_ = new long[] {1, 2, 3}", "C = 0", "R = C_[0]")

Nightlies: https://github.com/nbauernfeind/deephaven-core/actions/runs/8194327638

[kosak]

I think there is an increasing trend in the community to use less recursion, and I think I would recommend not using recursion here.

From what I can tell, you only recurse at most once, and I'm not sure if that's intentional or not. (I thought you had said during our meetings that you continually recurse so long as you are reducing the set of things that need to be recompiled, but I don't see that reflected here in the logic).

In any case, whether you had intended to recurse N levels or one level, I think the logic could be expressed more clearly by removing recursion. For example, code like this

    private void maybeCreateClassHelper(
            @NotNull final JavaCompiler compiler,
            @NotNull final JavaFileManager fileManager,
            @NotNull final List<CompilationRequestAttempt> requests,
            @NotNull final String rootPathAsString,
            @NotNull final String tempDirAsString,
            final int startInclusive,
            final int endExclusive) {
        final List<CompilationRequestAttempt> toRetry = new ArrayList<>();
        final boolean wantRetry = maybeCreateClassHelper2(compiler,
                fileManager, requests, rootPathAsString, tempDirAsString, startInclusive, endExclusive, toRetry);
        if (!wantRetry) {
            return;
        }

        final List<CompilationRequestAttempt> ignored = new ArrayList<>();
        final boolean wantRetry2 = maybeCreateClassHelper2(compiler,
                fileManager, toRetry, rootPathAsString, tempDirAsString, 0, toRetry.size(), ignored);

        if (wantRetry2) {
            // still sad, throw error or whatever you do here
        }
    }

    private boolean maybeCreateClassHelper2(
            @NotNull final JavaCompiler compiler,
            @NotNull final JavaFileManager fileManager,
            @NotNull final List<CompilationRequestAttempt> requests,
            @NotNull final String rootPathAsString,
            @NotNull final String tempDirAsString,
            final int startInclusive,
            final int endExclusive,
            List<CompilationRequestAttempt> toRetry) {
   ...

And the trick is that maybeCreateClassHelper2 gets a list that it can append to of things needing recompilation, and it can return true if it wants a recompilation to happen.

The above code only does one level of retry but can be easily rewritten to repeatedly retry.

[nbauernfeind]

There is continual resolution whenever there are collisions. This particular piece of code intentionally is doing two passes. The first pass might fail a few requests. If any request is failed, then none of the non-failing requests have their class files written. To properly complete these non-failures, we need to compile a second time to write the class files.

Thanks for the snippets; I've integrated your ideas into the solution.

[rcaudy]

Here is a code review focused just on QueryCompiler in PR form, since I reviewed offline:
nbauernfeind#4
Comments are prefixed RWC-CODE-REVIEW

[rcaudy]

Old pending review I forgot to submit

[rcaudy]

.

[rcaudy]

Maybe these should move to a "Conventions" class, but I'm not overly concerned.