Skip to content
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.

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

Deserialization is too slow (for the wrong reasons) #4558

Open
fieker opened this issue Feb 7, 2025 · 2 comments
Open

Deserialization is too slow (for the wrong reasons) #4558

fieker opened this issue Feb 7, 2025 · 2 comments

Comments

@fieker
Copy link
Contributor

fieker commented Feb 7, 2025

Try:

   save("/tmp/bla", identity_matrix(ZZ, 3000))
   load("/tmp/bla");

this takes (the load) > 30 sec. If you profile the load and look at the c-functions called, then >90% are spent in julia trying to figure out types. Less than 5% are json/ flint.

@lgoettgens
Copy link
Member

I had a quick look and it seems that load_object does have the intended return type as second argument, but in many cases cannot infer it itself (and thus has to figure out types in followup code with a huge performance loss). A simple patch like the following already speeds things up by 25% for me:

diff --git a/src/Serialization/main.jl b/src/Serialization/main.jl
index fff1110bf73..ffd10fee676 100644
--- a/src/Serialization/main.jl
+++ b/src/Serialization/main.jl
@@ -242,15 +242,15 @@ function load_typed_object(s::DeserializerState; override_params::Any = nothing)
   end
 end
 
-function load_object(s::DeserializerState, T::Type, key::Union{Symbol, Int})
+function load_object(s::DeserializerState, T::Type{innerT}, key::Union{Symbol, Int}) where innerT
   load_node(s, key) do _
-    load_object(s, T)
+    load_object(s, T)::innerT
   end
 end
 
-function load_object(s::DeserializerState, T::Type, params::Any, key::Union{Symbol, Int})
+function load_object(s::DeserializerState, T::Type{innerT}, params::Any, key::Union{Symbol, Int}) where innerT
   load_node(s, key) do _
-    load_object(s, T, params)
+    load_object(s, T, params)::innerT
   end
 end

But since there is a major overhaul in the works by @antonydellavecchia right now (see #4162), I would first wait for that and how it impacts performance, and then we can benchmark things like my patch again and include maybe something like this if it helps.

@antonydellavecchia
Copy link
Collaborator

I had a quick look and it seems that load_object does have the intended return type as second argument, but in many cases cannot infer it itself (and thus has to figure out types in followup code with a huge performance loss). A simple patch like the following already speeds things up by 25% for me:

diff --git a/src/Serialization/main.jl b/src/Serialization/main.jl
index fff1110bf73..ffd10fee676 100644
--- a/src/Serialization/main.jl
+++ b/src/Serialization/main.jl
@@ -242,15 +242,15 @@ function load_typed_object(s::DeserializerState; override_params::Any = nothing)
end
end

-function load_object(s::DeserializerState, T::Type, key::Union{Symbol, Int})
+function load_object(s::DeserializerState, T::Type{innerT}, key::Union{Symbol, Int}) where innerT
load_node(s, key) do _

  • load_object(s, T)
  • load_object(s, T)::innerT
    end
    end

-function load_object(s::DeserializerState, T::Type, params::Any, key::Union{Symbol, Int})
+function load_object(s::DeserializerState, T::Type{innerT}, params::Any, key::Union{Symbol, Int}) where innerT
load_node(s, key) do _

  • load_object(s, T, params)
  • load_object(s, T, params)::innerT
    end
    end
    But since there is a major overhaul in the works by @antonydellavecchia right now (see #4162), I would first wait for that and how it impacts performance, and then we can benchmark things like my patch again and include maybe something like this if it helps.

#4162 does such a fix.
Once this is done, we can work on removing parsing for small ints

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants