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

[FileFormats.MOF] Use JSON3 to write files #2613

Merged
merged 8 commits into from
Jan 17, 2025
Merged

[FileFormats.MOF] Use JSON3 to write files #2613

merged 8 commits into from
Jan 17, 2025

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.

test/FileFormats/MOF/MOF.jl Outdated Show resolved Hide resolved
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.

2 participants