Refactor `Binding` data structures in preparation for partition #54788

Keno · 2024-06-13T05:51:49Z

This is a re-worked extraction of #54654, adjusted to support the new semantics arrived at over the course of that thread. Note that this is the data-structure change only. The semantics in this PR still match current master to the greatest extent possible.

The core idea here is to split Binding in two: A new Binding with minimal data and a BindingPartition that holds all data that is world-age partitioned. In the present PR, these are always in 1:1 correspondednce, but after #54654, there will be multiple BindingPartitions for every Binding.

Essentially the owner and ty fields have been merged into a new restriction field of BindingPartition, which may also hold the value of a constant (consistent with the final semantics reached in #54654). The non-partitioned binding->value field is now used exclusively for non-constant globals. The disambiguation for how to interpret the restriction field happens via flags. ->imported grew to 2 bits and can now be one of NONE/IMPLICIT/ EXPLICIT/GUARD. GUARD corresponds to the old b->owner == NULL case. NONE corresponds to the old b->owner == b case, while IMPLICIT/EXPLICIT correspond to b->owner != b and the old imported flag. Other than that, the behavior of the flags is unchanged.

Additionally, fields are provided for min_world/max_world/next, but these are unused in this PR and prepratory only.

topolarity · 2024-06-13T14:12:57Z

src/codegen.cpp

-        jl_value_t *ty = jl_atomic_load_relaxed(&bnd->ty);
+    jl_binding_partition_t *bpart = jl_get_binding_partition(bnd, ctx.max_world);
+    if (bpart && !bpart->constp) {
+        jl_value_t *ty = bpart->restriction;
        if (ty != nullptr) {


I believe this needs additional guards now, for this case:

julia> global x::Int64 julia> foo() = (global x; x) julia> foo() # emits code w/ stores/loads directly to binding->value julia> const x = 15 # does not update binding->value any more julia> foo() # should be 15

Invalidation will take care of this.

topolarity · 2024-06-14T05:07:39Z

src/julia.h

+     * First look at ->imported (see above). If NONE, but `->constp`, this holds the
+     * value of the constant. Otherwise this is a global and holds the type restriction.
+     */
+    jl_value_t *restriction;


Would it be worth using Core.Const(...) here, instead of the constp logic?

I think that would be semantically consistent and reasonable, but there isn't really any benefit to doing so.

topolarity · 2024-06-14T05:18:09Z

src/julia.h

+    //  ->restriction does not hold any import-related data.
+    BINDING_IMPORT_NONE     = 0x0,
+    // Implicit: The binding was implicitly import from a `using`'d module.
+    //  ->restriction holds the imported binding


Is this still a short-circuit link to the defining module, or is it now the immediate module involved in the local using/import?

I'm assuming that we're expecting to maintain backlinks from all the intermediate modules that a binding was resolved through, so that a change to any of them splits all downstream binding partitions?

Yes, it short circuits. I haven't worked out the backedges yet.

I changed my mind on this. It no longer short circuits in the latest version.

topolarity · 2024-06-14T05:22:40Z

src/julia.h

    uint8_t constp:1;
    uint8_t exportp:1; // `public foo` sets `publicp`, `export foo` sets both `publicp` and `exportp`
    uint8_t publicp:1; // exportp without publicp is not allowed.
-    uint8_t imported:1;
+    uint8_t imported:2; // enum jl_binding_import
    uint8_t usingfailed:1;


Perhaps this should be BINDING_IMPORT_FAILED now

Keno · 2024-06-26T21:33:56Z

@nanosoldier runtests()

nanosoldier · 2024-06-27T08:04:59Z

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Keno · 2024-06-27T09:04:42Z

@nanosoldier runtests(["Tricks", "ComputationalResources", "ModuleInterfaceTools", "MultilineStrings", "ImportAll", "ForceImport", "InvariantsCore", "Syslogs", "TranscodingStreams", "EulerAngles", "MarkdownAST", "MD5", "StrFormat", "MiniLoggers", "JACC", "Invariants", "Automa", "BufferedStreams", "Setfield", "ClimaUtilities", "OctreeBH", "NonNegLeastSquaresMLJInterface", "HolidayCalendars", "KiwiConstraintSolver", "MixedStructTypes", "DistributedArrays", "MLJTSVDInterface", "HMMER", "CryptographicHashFunctions", "FASTX", "Neighborhood", "ShuffleProofs", "HOHQMesh", "Rocket", "JSON3", "TypedFASTX", "StrEntities", "OutlierDetectionTrees", "QuantumAlgebra", "SelfOrganizingMaps", "OneRule", "MLJDecisionTreeInterface", "LinearFold", "PAndQ", "ArtGallery", "OutlierDetectionNeighbors", "PkgJogger", "MultivariateMoments", "OnlineTechnicalIndicators", "ParallelKMeans", "ModuleInfo", "CharacteristicInvFourier", "MetaGraphsNext", "Strs", "BioRecordsProcessing", "GFF3", "ToolipsSession", "DataGraphs", "GAP", "BurrowsWheelerAligner", "TerminalUserInterfaces", "MLJClusteringInterface", "PreprocessMD", "InferenceObjects", "BibParser", "LazyAlgebra", "Bibliography", "CitationRecipes", "ChunkedCSV", "Infernal", "Logomaker", "NLopt", "CUBScout", "BlackBoxOptim", "MLJGLMInterface", "ExtendableGrids", "AbstractPPL", "StateSpaceEcon", "StaticWebPages", "DocumenterCitations", "KroneckerProductKernels", "Microbiome", "AbstractLogic", "Gaugefields", "COBREXA", "ScikitLearn", "BaytesOptim", "Bijectors", "Gtk", "DoseCalculators", "MLJLinearModels", "TwoDots", "MEstimation", "TuringCallbacks", "GtkSourceWidget", "MLJNaiveBayesInterface", "StatsLearnModels", "SliceSampling", "Ripserer", "MLJXGBoostInterface", "Baytes", "PhyloCoalSimulations", "DrugInteractions", "NonconvexMMA", "ClimaCoreSpectra", "MetidaNLopt", "QuartetNetworkGoodnessFit", "ProfileView", "ClimaCoreVTK", "OutlierDetectionNetworks", "TwoDotsModels", "PsychomotorVigilanceTask", "MLJEnsembles", "PhyloPlots", "MLJText", "MGVI", "MLJTestInterface", "OutlierDetectionTest", "MLJIteration", "ClimaCoreTempestRemap", "OutlierDetection", "PhyloGaussianBeliefProp", "RandomFeatures", "MLJParticleSwarmOptimization", "MCMCTempering", "YAXArrays", "MLJSerialization", "SpikeSorting", "MLJTuning", "ITensorGaussianMPS", "GenerativeTopographicMapping", "TenNetLib", "MicroCanonicalHMC", "EvoLinear", "PhyloDiamond", "GMMParameterEstimation", "PowerPlots", "ITensorVisualizationBase", "MAGEMinApp", "DecomposingPolynomialSystems", "MLJTestIntegration", "JointEnergyModels", "TrajGWAS", "MLJ", "BellDiagonalQudits", "GameTheory", "NonconvexJuniper", "NonconvexIpopt", "NonconvexPavito", "EqualitySampler", "RFFMakie", "PolynomialGTM", "SeisMakie", "GtkUtilities", "SimpleCrop", "SpectralStatistics", "PhyNEST", "ProbabilisticEchoInversion", "LeafGasExchange", "Garlic", "TaijaData", "ObservableCortex", "DiskArrayEngine", "MendelImpute", "SharedArrays", "QuadraticProgramNetworks"])

nanosoldier · 2024-06-27T10:59:59Z

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Keno · 2024-06-27T13:28:21Z

@nanosoldier runtests(["ModuleInterfaceTools", "Tricks", "Syslogs", "ComputationalResources", "JACC", "OctreeBH", "MarkdownAST", "Invariants", "StrFormat", "InvariantsCore", "HolidayCalendars", "KiwiConstraintSolver", "Neighborhood", "MiniLoggers", "FASTX", "ArtGallery", "CryptographicHashFunctions", "Automa", "TypedFASTX", "MultilineStrings", "ShuffleProofs", "ToolipsSession", "MD5", "TranscodingStreams", "TerminalUserInterfaces", "MixedStructTypes", "BurrowsWheelerAligner", "BioRecordsProcessing", "CitationRecipes", "ModuleInfo", "BibParser", "CUBScout", "AbstractPPL", "OutlierDetectionTrees", "LinearFold", "GFF3", "AbstractLogic", "Bibliography", "StaticWebPages", "PAndQ", "Strs", "Gtk", "DoseCalculators", "Setfield", "QuantumAlgebra", "TwoDots", "PkgJogger", "DataGraphs", "GtkSourceWidget", "OneRule", "NonNegLeastSquaresMLJInterface", "BlackBoxOptim", "MLJClusteringInterface", "MEstimation", "MLJTSVDInterface", "DrugInteractions", "Microbiome", "KroneckerProductKernels", "PreprocessMD", "PsychomotorVigilanceTask", "MetaGraphsNext", "TwoDotsModels", "ParallelKMeans", "Rocket", "Logomaker", "OutlierDetectionNeighbors", "ProfileView", "MLJTestInterface", "BaytesOptim", "MLJDecisionTreeInterface", "SelfOrganizingMaps", "HMMER", "MLJText", "DocumenterCitations", "MLJNaiveBayesInterface", "SpikeSorting", "MultivariateMoments", "OutlierDetectionTest", "MLJSerialization", "DistributedArrays", "Infernal", "PhyloCoalSimulations", "PhyloPlots", "ChunkedCSV", "MLJXGBoostInterface", "MLJGLMInterface", "QuartetNetworkGoodnessFit", "ExtendableGrids", "HOHQMesh", "StateSpaceEcon", "MLJIteration", "MLJParticleSwarmOptimization", "MLJTestIntegration", "Gaugefields", "SliceSampling", "MGVI", "MetidaNLopt", "OutlierDetection", "NLopt", "GMMParameterEstimation", "InferenceObjects", "MLJEnsembles", "SharedArrays", "DecomposingPolynomialSystems", "GtkUtilities", "COBREXA", "PhyloDiamond", "ScikitLearn", "ITensorGaussianMPS", "Bijectors", "MLJLinearModels", "EvoLinear", "PhyloGaussianBeliefProp", "JSON3", "NonconvexMMA", "YAXArrays", "BellDiagonalQudits", "MAGEMinApp", "MLJTuning", "LazyAlgebra", "StatsLearnModels", "TuringCallbacks", "OutlierDetectionNetworks", "GenerativeTopographicMapping", "TenNetLib", "ITensorVisualizationBase", "Ripserer", "MCMCTempering", "PhyNEST", "SeisMakie", "RandomFeatures", "JointEnergyModels", "EqualitySampler", "TaijaData", "ObservableCortex", "DiskArrayEngine", "SpectralStatistics", "NonconvexPavito", "MendelImpute", "LeafGasExchange", "RFFMakie", "PowerPlots", "SimpleCrop", "MLJ", "TrajGWAS", "Garlic", "GameTheory", "NonconvexJuniper", "NonconvexIpopt"])

nanosoldier · 2024-06-28T07:28:13Z

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Keno · 2024-08-04T03:25:26Z

@nanosoldier runtests(["Tricks", "ComputationalResources", "Syslogs", "JACC", "OctreeBH", "Neighborhood", "ToolipsSession", "Automa", "MixedStructTypes", "AbstractLogic", "TwoDots", "DoseCalculators", "GtkSourceWidget", "QuantumAlgebra", "Gtk", "PkgJogger", "TwoDotsModels", "MEstimation", "PsychomotorVigilanceTask", "DrugInteractions", "BlackBoxOptim", "Rocket", "ProfileView", "SpikeSorting", "ExtendableGrids", "MultivariateMoments", "DistributedArrays", "StateSpaceEcon", "MGVI", "SharedArrays", "GtkUtilities", "NonconvexMMA", "NLopt", "MetidaNLopt", "Gaugefields", "ScikitLearn", "LazyAlgebra", "JSON3", "RandomFeatures", "MLJTuning", "COBREXA", "BellDiagonalQudits", "TrajGWAS", "EqualitySampler", "PowerPlots", "MendelImpute", "GameTheory"])

nanosoldier · 2024-08-04T04:50:45Z

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Keno · 2024-08-04T05:35:48Z

@nanosoldier runtests(["Tricks", "Syslogs", "ComputationalResources", "JACC", "OctreeBH", "Neighborhood", "ToolipsSession", "Gtk", "DoseCalculators", "AbstractLogic", "Automa", "SharedArrays", "MEstimation", "MixedStructTypes", "DrugInteractions", "TwoDotsModels", "PsychomotorVigilanceTask", "SpikeSorting", "ProfileView", "BlackBoxOptim", "GtkUtilities", "PkgJogger", "MetidaNLopt", "NonconvexMMA", "MGVI", "Rocket", "StateSpaceEcon", "ScikitLearn", "BellDiagonalQudits", "Gaugefields", "NLopt", "COBREXA", "DistributedArrays", "PowerPlots", "TrajGWAS", "MultivariateMoments", "GameTheory", "ExtendableGrids", "MLJTuning", "EqualitySampler", "RandomFeatures", "LazyAlgebra", "MendelImpute", "JSON3", "QuantumAlgebra"])

nanosoldier · 2024-08-04T07:09:41Z

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Keno · 2024-08-10T02:10:48Z

@nanosoldier runtests(["Tricks", "Syslogs", "ComputationalResources", "JACC", "OctreeBH", "ToolipsSession", "SharedArrays", "MixedStructTypes", "PkgJogger", "MetidaNLopt", "NonconvexMMA", "BellDiagonalQudits", "ScikitLearn", "Gaugefields", "NLopt", "COBREXA", "DistributedArrays", "RandomFeatures", "GameTheory", "ExtendableGrids", "TrajGWAS", "PowerPlots", "MendelImpute", "JSON3", "QuantumAlgebra"])

nanosoldier · 2024-08-10T03:30:15Z

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Keno · 2024-08-12T03:24:21Z

Alright, I've restored all the old semantics, with the exception of

const x = 2
x = 3

and other attempts to redefine constants using setglobal!, which is now an error (and will be after full partition). Constant redefinition using the keyword, i.e.

const x = 2
const x = 3

and remains undefined behavior (for now).

I'm planning to address the atomics concerns and then merge this in the near future.

Keno · 2024-08-29T03:59:14Z

I think this should be good to go now. I've gone through and addressed the review comments. I'll run another round of pkgeval. Unless there's something pressing, I think I'll take anything additional post-commit since this is one of those branches that keeps accumulating conflicts and I need to keep the full partition branch up-to-date as well.

For compat with current semantics.

Keno · 2024-08-29T04:36:03Z

@nanosoldier runtests()

nanosoldier · 2024-08-29T05:17:03Z

Your job failed. Consult the server logs for more details (cc @maleadt).

Keno · 2024-08-29T05:21:51Z

@nanosoldier runtests()

nanosoldier · 2024-08-29T17:16:46Z

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

nsajko · 2024-08-30T18:03:33Z

This seems to break CI consistently: #55642

EDIT: also, the PkgEval above seems to find an "unreachable reached" situation in one or two packages

This codepath is odd. It derives a method name from the slotname metadata and then does an implicit assignment to that slot. Worse, as the comment indicates, the codepath is missing from the interpreter, which will crash if it were to ever encounter such a piece of code. I suspect this pattern is unused - I accidentally broke it badly in (the as of yet unmerged PR) #54788 and neither tests nor pkgeval noticed. So let's try removing it on master. If it turns out something does depend on it, we can go the opposite way and implement it properly in all the places that look at :method.

This is a re-worked extraction of #54654, adjusted to support the new semantics arrived at over the course of that thread. Note that this is the data-structure change only. The semantics in this PR still match current master to the greatest extent possible. The core idea here is to split `Binding` in two: A new `Binding` with minimal data and a `BindingPartition` that holds all data that is world-age partitioned. In the present PR, these are always in 1:1 correspondednce, but after #54654, there will be multiple `BindingPartition`s for every `Binding`. Essentially the `owner` and `ty` fields have been merged into a new `restriction` field of `BindingPartition`, which may also hold the value of a constant (consistent with the final semantics reached in #54654). The non-partitioned binding->value field is now used exclusively for non-constant globals. The disambiguation for how to interpret the `restriction` field happens via flags. `->imported` grew to 2 bits and can now be one of `NONE`/`IMPLICIT`/ `EXPLICIT`/`GUARD`. `GUARD` corresponds to the old `b->owner == NULL` case. `NONE` corresponds to the old `b->owner == b` case, while IMPLICIT/EXPLICIT correspond to `b->owner != b` and the old `imported` flag. Other than that, the behavior of the flags is unchanged. Additionally, fields are provided for `min_world`/`max_world`/`next`, but these are unused in this PR and prepratory only.

Keno requested review from vtjnash and topolarity June 13, 2024 05:51

Keno mentioned this pull request Jun 13, 2024

WIP: World-age partition bindings #54654

Open

14 tasks

topolarity reviewed Jun 13, 2024

View reviewed changes

Keno force-pushed the kf/constgloballowerrefactor branch 3 times, most recently from df97a48 to dfaa660 Compare June 14, 2024 06:40

topolarity reviewed Jun 14, 2024

View reviewed changes

Keno force-pushed the kf/constgloballowerrefactor branch 3 times, most recently from 499eed2 to f00f2e6 Compare June 19, 2024 23:33

Base automatically changed from kf/constgloballowerrefactor to master June 23, 2024 13:57

Keno force-pushed the kf/bindingstructrefactor branch 4 times, most recently from 890e668 to c2cc636 Compare June 26, 2024 21:01

Keno force-pushed the kf/bindingstructrefactor branch from aa4b302 to d1d842e Compare August 4, 2024 03:05

Keno added 8 commits August 28, 2024 21:21

Fix syntax test

9c8d0a0

Fix Revise

6bc267c

Address some review feedback

2788e72

Add jl_ prefix to ptr_kind_union

cf57f52

Address more review

e61dd9a

Revert some test changes not required for this PR

4ee82cf

Remove unused lpv

2737585

Rebase

e9023c6

Keno force-pushed the kf/bindingstructrefactor branch from 1bfe075 to e9023c6 Compare August 29, 2024 02:55

Keno added 2 commits August 29, 2024 03:32

Address final bits of code review

58be4d2

Fix typo

4df99a1

Consider "declared" to be "resolved"

6903b7c

For compat with current semantics.

Fix check

593a8d2

This comment was marked as resolved.

Sign in to view

Keno merged commit 7ea8a9a into master Aug 29, 2024
6 of 8 checks passed

Keno deleted the kf/bindingstructrefactor branch August 29, 2024 17:21

nsajko mentioned this pull request Aug 30, 2024

the syntax test suite is causing an assertion failure on master #55642

Closed

maleadt added a commit to JuliaGPU/GPUCompiler.jl that referenced this pull request Sep 17, 2024

Adapt to JuliaLang/julia#54788.

6b78eb0

maleadt added a commit to JuliaGPU/GPUCompiler.jl that referenced this pull request Sep 17, 2024

Adapt to JuliaLang/julia#54788.

47e8742

maleadt added a commit to JuliaGPU/GPUCompiler.jl that referenced this pull request Sep 17, 2024

Adapt to JuliaLang/julia#54788.

a96fc50

This was referenced Oct 23, 2024

drop require lock when not needed during loading to allow parallel precompile loading #56291

Merged

race-condition introduced by #54788 in the import_module function #56333

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor `Binding` data structures in preparation for partition #54788

Refactor `Binding` data structures in preparation for partition #54788

Keno commented Jun 13, 2024 •

edited

Loading

topolarity Jun 13, 2024

Keno Jun 14, 2024

topolarity Jun 14, 2024

Keno Jun 23, 2024

topolarity Jun 14, 2024

Keno Jun 23, 2024

Keno Jun 27, 2024

topolarity Jun 14, 2024

Keno Jun 23, 2024

Keno commented Jun 26, 2024

nanosoldier commented Jun 27, 2024

Keno commented Jun 27, 2024

nanosoldier commented Jun 27, 2024

Keno commented Jun 27, 2024

nanosoldier commented Jun 28, 2024

Keno commented Aug 4, 2024

nanosoldier commented Aug 4, 2024

Keno commented Aug 4, 2024

nanosoldier commented Aug 4, 2024

Keno commented Aug 10, 2024

nanosoldier commented Aug 10, 2024

Keno commented Aug 12, 2024

Keno commented Aug 29, 2024

Keno commented Aug 29, 2024

nanosoldier commented Aug 29, 2024

Keno commented Aug 29, 2024

This comment was marked as resolved.

nanosoldier commented Aug 29, 2024

nsajko commented Aug 30, 2024 •

edited

Loading

Refactor Binding data structures in preparation for partition #54788

Refactor Binding data structures in preparation for partition #54788

Conversation

Keno commented Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Keno commented Jun 26, 2024

nanosoldier commented Jun 27, 2024

Keno commented Jun 27, 2024

nanosoldier commented Jun 27, 2024

Keno commented Jun 27, 2024

nanosoldier commented Jun 28, 2024

Keno commented Aug 4, 2024

nanosoldier commented Aug 4, 2024

Keno commented Aug 4, 2024

nanosoldier commented Aug 4, 2024

Keno commented Aug 10, 2024

nanosoldier commented Aug 10, 2024

Keno commented Aug 12, 2024

Keno commented Aug 29, 2024

Keno commented Aug 29, 2024

nanosoldier commented Aug 29, 2024

Keno commented Aug 29, 2024

This comment was marked as resolved.

nanosoldier commented Aug 29, 2024

nsajko commented Aug 30, 2024 • edited Loading

Refactor `Binding` data structures in preparation for partition #54788

Refactor `Binding` data structures in preparation for partition #54788

Keno commented Jun 13, 2024 •

edited

Loading

nsajko commented Aug 30, 2024 •

edited

Loading