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

Fix: Improve concepts tree performance #794

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

Bilelkihal
Copy link
Collaborator

PR description

This PR provides partial improvements to the tree view performance. However, the primary bottleneck is in the backend, which requires specific endpoints optimized for the tree view, rather than relying on the default concept endpoints.

Changes

  • Optimized data retrieval in ConceptsController#index:
    The most time-consuming call when expanding a subtree is @concept = @ontology.explore.single_class({full: true}, params[:id])
    In this PR, it has been updated to @concept = @ontology.explore.single_class({display: 'prefLabel'}, params[:id]), as only the prefLabel and id are necessary at this level.
    (Note: This change provides a slight improvement in response time but does not fully resolve the delay.)

  • Removed redundant sorting:
    Sorting of concept children was duplicated both in the controller and in the component, adding approximately 1 second to load time.

@Bilelkihal Bilelkihal self-assigned this Oct 30, 2024
@Bilelkihal
Copy link
Collaborator Author

@syphax-bouazzouni this branch is already deployed to stage

@syphax-bouazzouni
Copy link
Collaborator

Hello @Bilelkihal, can you add a benchmark table with the before and after optimization, with cache enabled, and run it like 5 times, to see if it is better or not?
Here is how

require 'benchmark'

iterations = 5
total_time = 0

Benchmark.bmbm do |bm|
  bm.report("tree:") do
    iterations.times do
      time = Benchmark.measure do
        # your code to benchmark
      end
      total_time += time.real
    end
  end
end 

average_time = total_time / iterations
puts "Average execution time over #{iterations} runs: #{average_time.round(5)} seconds"

And finally can you open an issue to describe what optimization need to be done in the API side.

@syphax-bouazzouni
Copy link
Collaborator

@syphax-bouazzouni this branch is already deployed to stage

@Bilelkihal I tested it in Stageportal. The tree performance seems fine, but we still have some latency when we select a concept. The concept takes time to show the details.

Screencast.from.2024-10-30.10-50-52.webm

@Bilelkihal
Copy link
Collaborator Author

Hello @Bilelkihal, can you add a benchmark table with the before and after optimization, with cache enabled, and run it like 5 times, to see if it is better or not? Here is how

require 'benchmark'

iterations = 5
total_time = 0

Benchmark.bmbm do |bm|
  bm.report("tree:") do
    iterations.times do
      time = Benchmark.measure do
        # your code to benchmark
      end
      total_time += time.real
    end
  end
end 

average_time = total_time / iterations
puts "Average execution time over #{iterations} runs: #{average_time.round(5)} seconds"

And finally can you open an issue to describe what optimization need to be done in the API side.

Benchmark (stage api)

AGROVOC Tree (cache disabled)

Concept Before After
Activities 3.05s 2.54s
Administration 2.34s 1.28s
Analysis 1.30s 1.70s
Application 2.20s 1.37s
Cleaning 3.30s 2.7s

I don't see a big difference, but in general it's better.

@syphax-bouazzouni syphax-bouazzouni merged commit d67ba52 into development Oct 30, 2024
0 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants