Skip to content

Conversation

@joaquimg
Copy link
Member

A test

using JuMP

model = Model()
@variable(model, x[1:1000] >= 0)
@constraint(model, y[1:1000], sum(i*x[i] for i in 1:1000) == 1)

for i in 1:5
    GC.gc()
    @time write_to_file(model, "test.mof.json")
end

Results:

before PR:

  3.312154 seconds (5.14 M allocations: 772.433 MiB, 6.80% gc time)
  3.546777 seconds (5.14 M allocations: 772.416 MiB, 6.70% gc time)
  3.368654 seconds (5.14 M allocations: 772.483 MiB, 6.37% gc time)
  3.924556 seconds (5.14 M allocations: 772.422 MiB, 6.55% gc time)
  3.771463 seconds (5.14 M allocations: 772.390 MiB, 6.51% gc time)

after PR:

  0.965306 seconds (9.63 M allocations: 577.394 MiB, 7.00% gc time)
  1.171944 seconds (9.63 M allocations: 577.394 MiB, 6.04% gc time)
  1.028857 seconds (9.63 M allocations: 577.394 MiB, 5.34% gc time)
  1.126085 seconds (9.63 M allocations: 577.394 MiB, 5.80% gc time)
  1.120570 seconds (9.63 M allocations: 577.394 MiB, 5.72% gc time)

@odow
Copy link
Member

odow commented Jan 16, 2025

I'm not against this, but I'm not strongly in favor. Do you have a benchmark of the Sienna instance?

@odow
Copy link
Member

odow commented Jan 16, 2025

If we do this, we should use JSON3 to replace all instances of JSON. Let's not have two packages floating around just for the sake of it.

Copy link
Member

@odow odow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I did some tests locally with JSON3, and I'm happy now.

@odow odow merged commit 70b3c83 into master Jan 17, 2025
16 checks passed
@odow odow deleted the jg/json3 branch January 17, 2025 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants