Skip to content

make just one MethodTable #58131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

make just one MethodTable #58131

wants to merge 1 commit into from

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Apr 15, 2025 •
edited
Loading

Most of the complexity here is because of the need to provide legacy support to the experimental external/overlay MethodTable, otherwise this would likely just be a simple renaming of Table->Cache, which would then have simply been unnecessary and made this PR very tiny. Oh well.

Fixes #57560

The remaining question is what to call this global singleton object for introspection. I currently use Core.GlobalMethods. the placeholder name Core._, but someone may object to that.

The remaining failing test (in core) is due to the better optimized cache structure here being better specialized and optimized now, but not yet reimplementing the fallback we had before for that one specific case. The good news is that there is no longer any reason to limit this to that one specific case anymore, since we could now fix all similar cases too and make it generally applicable to them. The bad news is that we might need every supertype to contain a list of all of its possible subtypes so that we can efficiently walk to all of their caches, if we want to restore that case to functional.

TODO:

@StefanKarpinski
Copy link
Member

The bad news is that we might need every supertype to contain a list of all of its possible subtypes so that we can efficiently walk to all of their caches, if we want to restore that case to functional.

This doesn't seem that awful. Would obviously speed up subtypes too, which is currently very slow.

@JeffBezanson
Copy link
Member

Wow, did not expect this to get implemented so fast! Benchmarking this will be interesting (by which I mean, I hope it's boring :) ).

Agree that keeping lists of subtypes seems ok. Though I'm sure someone out there is constructing arbitrarily-long chains of subtypes in a loop 😂

but not yet reimplementing the fallback we had before for that one specific case

I do think we'll need this; I'm pretty sure people use this feature. If it helps, we can shard the global method table based on some properties of the signature; no reason the single method table actually has to be a single MethodTable object.

@vtjnash vtjnash force-pushed the jn/GlobalMethodTable branch 2 times, most recently from 6467c56 to 5f6efef Compare April 23, 2025 20:22
@vtjnash vtjnash mentioned this pull request Apr 24, 2025
Merged
@vtjnash vtjnash force-pushed the jn/GlobalMethodTable branch from 5f6efef to c4d3ce8 Compare April 24, 2025 20:42
@KristofferC
Copy link
Member

KristofferC commented Apr 25, 2025 •
edited
Loading

I currently use the placeholder name Core._, but someone may object to that.

17455884561771762396181280348029

_ is not allowed to be used as an rvalue so it feels weird to use it. Just call it METHOD_TABLE or something? Why make it so obscure?

vtjnash added a commit that referenced this pull request Apr 25, 2025
@vtjnash
optimize Type method table queries and insertions (#58216)
d992554 
Ensure a total split of constructors and non-constructors, so they do
not end up seaching the opposing table. Instead cache all kind lookups
as keyed by typename(Type). Not really needed on its own, but it avoids
a minor performance regression in
#58131.
KristofferC pushed a commit that referenced this pull request Apr 29, 2025
@vtjnash @KristofferC
optimize Type method table queries and insertions (#58216)
bef1824 
Ensure a total split of constructors and non-constructors, so they do
not end up seaching the opposing table. Instead cache all kind lookups
as keyed by typename(Type). Not really needed on its own, but it avoids
a minor performance regression in
#58131.

(cherry picked from commit d992554)
LebedevRI pushed a commit to LebedevRI/julia that referenced this pull request May 2, 2025
@vtjnash @LebedevRI
optimize Type method table queries and insertions (JuliaLang#58216)
da6fbaa 
Ensure a total split of constructors and non-constructors, so they do
not end up seaching the opposing table. Instead cache all kind lookups
as keyed by typename(Type). Not really needed on its own, but it avoids
a minor performance regression in
JuliaLang#58131.
@vtjnash vtjnash mentioned this pull request May 2, 2025
Open
@vtjnash vtjnash force-pushed the jn/GlobalMethodTable branch from c4d3ce8 to 0e51bdf Compare May 8, 2025 18:01
@vtjnash
make just one MethodTable
898e1c1 
Most of the complexity here is because of the need to provide legacy
support to the experimental external/overlay MethodTable, otherwise this
would just be a simple renaming of Table->Cache, which would then have
simply been unnecessary and made this PR very tiny. Oh well.
@vtjnash vtjnash force-pushed the jn/GlobalMethodTable branch from 0e51bdf to 898e1c1 Compare May 14, 2025 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

methodtable total searches need to consider all binding partitions
4 participants
@vtjnash @StefanKarpinski @JeffBezanson @KristofferC