Remove Access::Custom; it forces table scans to figure out what can run

[rsheeter]

Access::Custom took a closure and required a scan over pending jobs to determine if it matched any. Instead count the number of pending jobs of each type and resolve "blocks on all of type" (e.g. all IR Glyphs) by checking if the count of those pending is 0.

Key outcome is deletion of can_run_scan from fontc/src/workload.rs . hyperfine reports no significant change in performance:

$ cargo build --release && hyperfine --warmup 25 --runs 100 --prepare 'rm -rf build/' 'target/release/fontc ../OswaldFont/sources/Oswald.glyphs'
main   130.6 ms ±  10.9 ms
branch 127.9 ms ±   8.0 ms

A mildly amusing outcome is that you can potentially detect that all-of(something) is done before you can detect a specific id is done because the former is a decrement to an atomic and the latter requires a message to pass through a channel.

I feel like there should be a nicer way than how I've rigged AllOrOne but I haven't pinned it down.

[cmyr]

It feels like there is an awful lot of complexity here, and it is not clear to me how much of that complexity is inherent versus accumulated. Are you happy with the state of things right now? Are there any particular bits that you find annoying?

I have a hunch that there is a better way to represent this concept of a 'work id' in the type system, and to do so in such a way that lets us delete a lot of code.

I also understand that the glyphs/gvar ids are tricky, because unlike all of the other work tasks these can't be known at compile time, as the set of glyphs obviously depends on the source format.

[rsheeter]

Are there any particular bits that you find annoying?

Due to lack of a way to get a discriminant other than from an instance you end up with this AllOrOne::All(AnyWorkId::AllOfBe(WorkId::GlyfFragment(GlyphName::NOTDEF))) which misleadingly has to specify a specific glyph name. It's also more nested than I'd like but I'm not sure how to avoid that while retaining separate enums for FE vs BE work.

I would also like to make One only applicable to variants that do not contain data or to have a way to identify that and treat One(variant without data) as equivalent to All(variant without data) because All can resolve completion by simply checking a counter.

EDIT: I added AccessBuilder to reduce the noise of examples like the one above and hopefully make it clearer when the match is on the variant vs on the variant + the data for the exemplar.

[rsheeter]

Rebased to resolve conflict with main

[cmyr]

I think that the builder is a nice addition, but I also think there is a lot of complexity around this access/workid abstraction and I'm not sure that complexity is well spent.

That feels beyond the scope of this PR, though, so I'm happy to see this get in if it is improving performance in some meaningful way.

[rsheeter]

I'm not sure that complexity is well spent

Do you have a specific suggestion? - it seems at least somewhat required to me. You need an identity for tasks to be able to say A depends on B. You could dump access checks but that opens the door for inconsistently reproducible ordering problems.

[cmyr]

it's hard to sort of tease apart the various parts of the current system to arrive at concrete suggestions, but from some earlier experimentation I did develop some ideas I found useful.

• conceptualize everything in terms of artifacts/resources (produced by work) and work (which produces artifacts)
• reframe read/write access more explicitly in terms of the dependencies and outputs of work
• instead of describing these dependencies in terms of the work (e.g., instead of WorkId, dependencies should be described in terms of the actual resources/artifacts, with something like a ResourceId.)
• make the declaration of dependencies more dynamic, to account for the fact that (e.g.) you don't know the set of glyphs you are going to be compiling until you've started
• in combination, these things would let us say things like "this task depends on the existence of a particular glyph outline", or "this task requires all glyph outlines", where there would be a separate resource id for each individual glyph, as well as a resource id for 'all of the outlines'.

I also think that existence of separate id types and context types for fe/be is unnecessary, and these could be unified. I also-also think that there are better ways to express the concept of an id, in the type system, than with an enum. I was playing around a bit with the idea of actually just making this a struct that would store a type id and an optional 'instance identifier', so that we are actually reusing the type of the resource as the identifier.

But all of this is complicated, and hard to be sure about without really fleshing out at least a toy implementation, and it is certainly beyond the scope of this PR.

[rsheeter]

The majority of what you are describing is already the case.

dependencies should be described in terms of the actual resources/artifacts

The work identifier specifies an asset or resource (naming is the hardest part) more than actual work given than one work can both consume and produce many things so this is true other than the name not being resource or artifact or asset or w/e. Maybe we should rename things.

I also think that existence of separate id types and context types for fe/be is unnecessary, and these could be unified

It's possible but I'm of the opinion it would be harmful. The phases are deliberately separated and some errors, such as a FE task trying to use a BE output, won't compile.

"this task depends on the existence of a particular glyph outline", or "this task requires all glyph outlines", where there would be a separate resource id for each individual glyph, as well as a resource id for 'all of the outlines'.

This is true already

a struct that would store a type id and an optional 'instance identifier'

The set of possible ids is known in advance. Which ones have an instance identifier is known in advance. To me that's describing an enum. What would the advantage be of this change?

[cmyr]

The majority of what you are describing is already the case.

As with most of my changes, I'm not suggesting a drastic departure from where we are right now, just a slightly different way of encoding things in the type system. My dream is an end state where we have a Fontc type which we configure with some input arguments, and then we are able to (in one or two lines) say 'now give me the ir::Kerning for this input', and the compiler can figure out how to resolve all of its dependencies internally, without needing us to manually create any work or otherwise do any per-execution setup.

dependencies should be described in terms of the actual resources/artifacts

The work identifier specifies an asset or resource (naming is the hardest part) more than actual work given than one work can both consume and produce many things so this is true other than the name not being resource or artifact or asset or w/e. Maybe we should rename things.

Agree that this is largely a matter of naming.

I also think that existence of separate id types and context types for fe/be is unnecessary, and these could be unified

It's possible but I'm of the opinion it would be harmful. The phases are deliberately separated and some errors, such as a FE task trying to use a BE output, won't compile.

I'm unconvinced this is a problem in practice. In the world I am imagining, a type would still only be able to access the things that it had named as dependencies. A FE task would not be able to name a BE resource, and so couldn't request it.

"this task depends on the existence of a particular glyph outline", or "this task requires all glyph outlines", where there would be a separate resource id for each individual glyph, as well as a resource id for 'all of the outlines'.

This is true already

If it is, I have misunderstood the point of this PR, which appeared to be sort of bolting an 'all of these things' concept onto the existing workid idea, which is different from having WorkId::AllGlyphs (which is what i'm suggesting) so I'm probably overlooking something?

a struct that would store a type id and an optional 'instance identifier'

The set of possible ids is known in advance. Which ones have an instance identifier is known in advance. To me that's describing an enum. What would the advantage be of this change?

So the rationale here is based on the idea of having a single context which doesn't actually know what it is storing, and is shared between fe/be. In this world the set of possible ids is not known ahead of time, at least not in the place where the context is declared. Basically instead of having a context with named fields, the context would wrap some sort of map type that would store type-erased resources, with the key being this struct. This is hard to describe in prose, but imagine something like,

/// Stores all the intermediate state
pub(crate) struct ResourceStore {
    storage: HashMap<ResourceIdentifier, AnyResource>,
}

pub(crate) struct AnyResource {
    inner: Arc<dyn Any>,
    // just used for debugging
    type_name: &'static str,
}

#[derive(Clone, Debug, Eq, PartialEq, Hash)]
pub struct ResourceIdentifier {
    /// The type of the generated resource
    type_id: TypeId,
    /// For resources with multiple instances (glyphs, e.g.) some extra token
    /// that uniquely identifies a specific instance
    instance_id: (), //TODO
}

impl ResourceStore {
    pub fn get_any(&self, ident: &ResourceIdentifier) -> Option<&AnyResource> {
        self.storage.get(ident)
    }

    pub fn get<T: 'static>(&self) -> Result<&T, ResourceError> {
        self.get_any(&ResourceIdentifier::<T>::new())
            .ok_or_else(|| ResourceError::Missing(std::any::type_name::<T>()))
            .and_then(AnyResource::downcast)
    }
}

Here the ResourceStore type is doing the job of holding onto all the precomputed resources. This is just a sketch, but the basic idea is that you can write,

let kerning: ir::Kerning = context.get()?;

and it 'just works'.

Again, this is just one idea, and there may be serious problems to it, but at some point it could be an interesting idea to explore.

[rsheeter]

The code example helps, ty. I was imagining our current contexts glued together which implies everything can see everything. The sketch neatly avoids that.

I agree it feels like there might be something nicer than what we do currently here (assuming no blocker exists going from sketch to real implementation). I'm a bit concerned that the ROI would be low. It would be a lot of work and the benefits I see are:

1. A specific type of test is easier
2. Adding fields to Context is easier, we are spared some repetitive manual changes

Are there other benefits?

an end state where we have a Fontc type which we configure with some input arguments, and then we are able to (in one or two lines) say 'now give me the ir::Kerning for this input', and the compiler can figure out how to resolve all of its dependencies internally

This part seems strongly informed by the problem you encountered in writing a test where we create and run specific work rather awkwardly. Those tests are poorly written, we should probably go fix them. However, I'm not immediately certian that a non-test users needs the "run to an arbitrary point" capability?

Thinking "aloud," I think we could make the test that seems to inform the end state easier through a less invasive change. For example, to run until a specific thing is produced you would, roughly:

1. Make a method to process [an input] until you have produced [a work id], or perhaps a set of ids so I can write a test that says "run until you produce {glyph ir for these glyphs}". That method would:
   1. Call fontc::create_workload to generate a full graph
   2. Call a new method to prune the workload down to only things you need to produce let's say fontir::WorkId::Kerning
      • this would start at the work for kerning, look at it's read access, and close over work reachable by read access to build the set of ids retained
      • then dump all the ids that aren't retained
   3. Execute the workload
   4. Return context
2. You read your item out of Context

Not as elegant as let kern: Kern = context.get()? but readily achievable in a few hours.

without needing us to manually create any work or otherwise do any per-execution setup.

You need something to create a graph of work, and to update it at runtime because the graph is not a pure function of source but changes in response to operations on that source, such as to generate a new glyph based on what we found in the glyphs of the source. You've still got "work," you still need to know what work depends on, and you've still got something responsible for executing a workload.

Hm. In the sketched model whose job is it to create the work to build the things you need to provide the ir::Kerning? You need to know what it reads, transitively, and to assemble a graph. Getting that graph right is arguably the interesting part.

[cmyr]

The code example helps, ty. I was imagining our current contexts glued together which implies everything can see everything. The sketch neatly avoids that.

I agree it feels like there might be something nicer than what we do currently here (assuming no blocker exists going from sketch to real implementation). I'm a bit concerned that the ROI would be low. It would be a lot of work and the benefits I see are:

1. A specific type of test is easier
2. Adding fields to Context is easier, we are spared some repetitive manual changes

Are there other benefits?

The main benefits as I see them (just for this context sketch):

• easier to write tests: I actually think this is a really valuable property; reducing the friction around writing up a new simple test case can go a long way to improving our robustness long term.
• better encapsulation: we can have a context type that doesn't need to know everything that it's storing, which is a nice API boundary, and the public API surface will be very small. Adding new resources is simple, and there's no confusion about what context I'm using etc.
• better maintainability: less code, fewer types, one clear idea. The implementation will need to be documented, but it should be easy enough for a new contributor to go do a quick skim of the types involved and understand what's going on.
• the API itself enforces our constraints: by this I mean that you can only get resources out of the context that you can name. It would be easy enough to also layer an access control mechanism on top of this, where you are only allowed to access items that were listed in your read_access/dependency list, but at an even more fundamental level you don't see all the fields in the context, so you couldn't accidentally call the wrong one.

an end state where we have a Fontc type which we configure with some input arguments, and then we are able to (in one or two lines) say 'now give me the ir::Kerning for this input', and the compiler can figure out how to resolve all of its dependencies internally

This part seems strongly informed by the problem you encountered in writing a test where we create and run specific work rather awkwardly. Those tests are poorly written, we should probably go fix them. However, I'm not immediately certain that a non-test users needs the "run to an arbitrary point" capability?

Less than "run to an arbitrary point" I've been thinking of this as "generate this arbitrary artifact". In the test example the artifact might be some IR, but an artifact might also be an instance or a subset or something else. The idea is that there is a single mechanism/abstraction through which resources are resolved, regardless of what point in the program graph you find yourself. I think this has a very nice conceptual simplicity.

Thinking "aloud," I think we could make the test that seems to inform the end state easier through a less invasive change. For example, to run until a specific thing is produced you would, roughly:

I think that any changes we might make should be done in stages, and I think that there are reasonable ways to break it up. For instance, I think that we could probably unify the contexts without changing too much else, and iterate from there. I definitely agree that anything we can do in the immediate term that improves testing would be great.

Not as elegant as let kern: Kern = context.get()? but readily achievable in a few hours.

without needing us to manually create any work or otherwise do any per-execution setup.

You need something to create a graph of work, and to update it at runtime because the graph is not a pure function of source but changes in response to operations on that source, such as to generate a new glyph based on what we found in the glyphs of the source. You've still got "work," you still need to know what work depends on, and you've still got something responsible for executing a workload.

Hm. In the sketched model whose job is it to create the work to build the things you need to provide the ir::Kerning? You need to know what it reads, transitively, and to assemble a graph. Getting that graph right is arguably the interesting part.

Right, I think you're definitely identifying the right questions here.

I have thought about this a reasonable amount, and I have some ideas. I'll try to sketch out at a high level how I've imagined this whole system fitting together:

• as mentioned, we have a clear distinction between 'resources' and 'work'.
• we have a trait that looks a lot like the current Work trait.
• (as an aside, I picture all of the types and core functionality related to the plumbing and logic of the compiler living somewhere like fontdrasil, e.g. somewhere outside of the be/ir crates, that is a dependency of everything)
• we have a new trait called, say, ResourceProvider. The various frontends, as well as the backend, would provide types that implement this trait. It will have an API like,

  /// A type that is capable of creating tasks that produce resources.
  trait ResourceProvider {
      fn provide_work(&self, for_resource: &ResourceIdentifier) -> Option<AnyWork>;
  }

• There will be some sort of compilation context that owns the graph and manages execution.
• Compilation starts when some root resource is requested, and the compiler is provided some input arguments (sources e.g.)
• based on the inputs to the compiler, it configures some (small) set of resource providers. For the sake of simplicity let's just imagine there are always two, one for the frontend and one for the backend.
• The process of building the graph is recursive:
  • the compiler looks at the requested resource, and checks if it exists.
  • if the resource exists, we're done.
  • if the resource does not exist, then the compiler asks each ResourceProvider if they can provide a Work that will generate it.
  • (it is an invariant that only one provider can exist for any resource, and there should always be a provider for any resource, although we will error nicely if this is violated)
  • we inspect the provided Work items dependencies, recursing for each one.
  • we use this to build a graph of required work, which we execute.

So that's a hand-wavy sketch. The devil is in the details; for instance in some cases (e.g. glyphs) we do not know all of the resources we need (assuming one per glyph) until we have done some preliminary work, and so some Work items cannot actually name their dependencies until some other work has completed (getting the glyph order, e.g.) and so I'd also want some mechanism for Work to basically be polled for dependencies: the Work item could signal from its dependencies/access method that it wasn't ready yet, and should be called back after some preliminary dependency had been resolved, or something.

And then all of this would be rolled up in a nice Compiler struct of some kind that would own the runloop and handle things like tracing execution and updating the graph as work completed.

And again, this is all just ideas, and although I have played around with implementing some of this there are lots of unanswered questions; but I do think that figuring out a scalable & ergonomic architecture would offer some significant long term benefits.

I also think that we can get here iteratively. Ultimately most of the important logic in fontc is the stuff in the exec methods, and none of that logic really needs to change (outside of the stuff that directly implicates the current architecture, like getting/setting fields on the contexts.)

Indeed, in a hypothetical world where we wanted to explore these changes, the route I would propose would be to first refactor exec so that all the business logic is in independent fns that know nothing about fontc, and are just passed arguments and return a result, with the exec method just being used to get the arguments and set the results on the context; that way we could have the core of this new compiler exist alongside the current one, and it could reuse all the current logic.