Skip to content

Conversation

wllgrnt
Copy link

@wllgrnt wllgrnt commented Jan 18, 2023

Motivation and Context

This change removes the notion of invalid and valid modules, by only deserialising, linking, and validating the global variables necessary for the function currently being loaded. Also fixes #417, which the serialisation of the globals exposed.

Approach

  1. Alter the compiler cache interface so that only the TypedCallTarget asked for by the cache is returned. Requires a change to
  2. Double-serialise the globals
  3. Store a function dependency graph and global dependency dictionary, and save these to disk.
  4. Keep a tally of which functions have linked+validated globals and are thus allowed to run (i.e have function pointers that won't segfault).
  5. Give native_ast_to_llvm access to the compiler cache for use in namedCallTargetToLLVM

How Has This Been Tested?

One new test in compiler_cache_test and alterations to two others.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@wllgrnt wllgrnt force-pushed the will-gus-partial-compiler-load branch 3 times, most recently from 1618ac7 to 640eac5 Compare January 25, 2023 18:51
@wllgrnt wllgrnt force-pushed the will-gus-partial-compiler-load branch from 640eac5 to 9b27dc7 Compare January 26, 2023 15:50
Current implementation of the interface, used via
_loadFromCompilerCache, runs loadForSymbol for a given link name and
then returns two dicts representing everything the cache touched during
this load process. Maintaining this interface makes the partial-load
refactor difficult, and muddies the converter layers/cache relationship.
Here we alter the API so that we can ask the cache for
a TypedCallTarget, and add modules, and that's it. This means getting
rid of _loadFromCompilerCache, and associated registers for tracking
what's being converted. Also means passing the cache down to the
native_ast_to_llvm layer.
@wllgrnt wllgrnt force-pushed the will-gus-partial-compiler-load branch from 03a3809 to b8f7628 Compare February 8, 2023 22:36
Previously we would always attempt to link and validate all global
variables when loading a module from the cache. This caused linking
errors, or validation errors, or deserialization errors, and meant we
needed the mark_invalid mechanism for handling modules with outdated
global variables. Here we add double-serialised global variables, and
only deserialize,link&validate the subset required for the function
required (and its dependencies). This requires the cache to store
a function and global_var dependency graph. Also add utility methods
for GlobalVariableDefinition.
@wllgrnt wllgrnt force-pushed the will-gus-partial-compiler-load branch 3 times, most recently from a3d7d76 to 641fa93 Compare February 10, 2023 16:33
Pickle supports a protocol where __reduce__returns a string giving
the global name. Implementing this behaviour lets us serialize numpy
ufuncs. Also adjust installInflightFunctions to handle new load
behaviour, fix an instability caused by not leaving LoadedModule
objects in memory, and adjust alternative test.
@wllgrnt wllgrnt force-pushed the will-gus-partial-compiler-load branch from 641fa93 to b58b173 Compare February 14, 2023 20:45
@wllgrnt wllgrnt requested a review from braxtonmckee February 14, 2023 22:02
@braxtonmckee braxtonmckee merged commit 22f01c5 into dev Feb 15, 2023
@wllgrnt wllgrnt deleted the will-gus-partial-compiler-load branch February 15, 2023 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SerializationContext cannot serialize numpy ufuncs
2 participants