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

Make _metadata regular JSONType field #11902

Merged

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Apr 27, 2021

What did you do?

  • Make _metadata regular JSONType field

Why did you make this change?

#11885 made it very obvious that there is a lot of overhead with the nested mutable json tracking. I believe setting metadata doesn't really require the nested tracking that the MutableJSONType field gives us.

How to test the changes?

(select the most appropriate option; if the latter, provide steps for testing below)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

For UI Components

  • I've included a screenshot of the changes

@mvdbeek
Copy link
Member Author

mvdbeek commented Apr 27, 2021

This is together with #11901, closing and expanding the test dataset 6 times from #11885. :

make_html_table (lib/galaxy/datatypes/tabular.py:131)
function called 6 times

         12132 function calls in 0.011 seconds

   Ordered by: cumulative time, internal time, call count

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        6    0.000    0.000    0.011    0.002 tabular.py:131(make_html_table)
        6    0.002    0.000    0.008    0.001 tabular.py:191(make_html_peek_rows)
     1800    0.003    0.000    0.005    0.000 {built-in method markupsafe._speedups.escape}
        6    0.001    0.000    0.003    0.001 tabular.py:144(make_html_peek_header)
       54    0.001    0.000    0.003    0.000 metadata.py:119(__getattr__)
     1800    0.001    0.000    0.002    0.000 __init__.py:70(__new__)
      114    0.000    0.000    0.001    0.000 metadata.py:87(spec)
      114    0.000    0.000    0.001    0.000 __init__.py:2838(datatype)
      114    0.000    0.000    0.001    0.000 __init__.py:2720(datatype_for_extension)
     1800    0.000    0.000    0.000    0.000 {built-in method __new__ of type object at 0x10fdea420}
       54    0.000    0.000    0.000    0.000 session.py:3453(object_session)
     2490    0.000    0.000    0.000    0.000 {method 'append' of 'list' objects}
       60    0.000    0.000    0.000    0.000 __init__.py:2842(get_metadata)
      336    0.000    0.000    0.000    0.000 metadata.py:75(get_parent)
      234    0.000    0.000    0.000    0.000 attributes.py:286(__get__)
     1860    0.000    0.000    0.000    0.000 {built-in method builtins.hasattr}
       54    0.000    0.000    0.000    0.000 session.py:45(_state_session)
      300    0.000    0.000    0.000    0.000 {method 'format' of 'str' objects}
       54    0.000    0.000    0.000    0.000 metadata.py:364(wrap)
       30    0.000    0.000    0.000    0.000 {method 'split' of 'str' objects}
      114    0.000    0.000    0.000    0.000 registry.py:579(get_datatype_by_extension)
       54    0.000    0.000    0.000    0.000 weakref.py:131(__getitem__)
      114    0.000    0.000    0.000    0.000 __init__.py:132(_get_datatypes_registry)
       12    0.000    0.000    0.000    0.000 __init__.py:2779(peek)
       18    0.000    0.000    0.000    0.000 {method 'join' of 'str' objects}
      120    0.000    0.000    0.000    0.000 {method 'get' of 'dict' objects}
       48    0.000    0.000    0.000    0.000 {built-in method builtins.min}
      114    0.000    0.000    0.000    0.000 {method 'lower' of 'str' objects}
        6    0.000    0.000    0.000    0.000 {method 'splitlines' of 'str' objects}
       30    0.000    0.000    0.000    0.000 {method 'startswith' of 'str' objects}
      102    0.000    0.000    0.000    0.000 {built-in method builtins.len}
       54    0.000    0.000    0.000    0.000 metadata.py:320(wrap)
       42    0.000    0.000    0.000    0.000 {built-in method builtins.isinstance}
        6    0.000    0.000    0.000    0.000 {method 'extend' of 'list' objects}
        6    0.000    0.000    0.000    0.000 {method 'items' of 'collections.OrderedDict' objects}
        6    0.000    0.000    0.000    0.000 {method 'disable' of '_lsprof.Profiler' objects}

and in the UI there is no noticeable delay when expanding the dataset.

@mvdbeek mvdbeek force-pushed the try_dropping_metadatatype_for_hda_metadata branch from 5eb3231 to 3369293 Compare April 27, 2021 13:49
@mvdbeek
Copy link
Member Author

mvdbeek commented Apr 27, 2021

Well, that's a nice set of unrelated errors ... really need to clean up those failing tests. But looks like none of these are related to this PR.

@mvdbeek mvdbeek marked this pull request as ready for review April 27, 2021 16:06
@github-actions github-actions bot added this to the 21.05 milestone Apr 27, 2021
@mvdbeek mvdbeek requested a review from jmchilton May 3, 2021 11:31
@dannon dannon merged commit c3fc591 into galaxyproject:dev May 4, 2021
mvdbeek added a commit to mvdbeek/galaxy that referenced this pull request Feb 7, 2022
@mvdbeek mvdbeek mentioned this pull request Feb 7, 2022
5 tasks
mvdbeek added a commit to mvdbeek/galaxy that referenced this pull request Feb 7, 2022
@nsoranzo nsoranzo deleted the try_dropping_metadatatype_for_hda_metadata branch February 7, 2022 13:01
mvdbeek added a commit to mvdbeek/galaxy that referenced this pull request Feb 7, 2022
mvdbeek added a commit to mvdbeek/galaxy that referenced this pull request Feb 11, 2022
astrovsky01 pushed a commit to astrovsky01/galaxy that referenced this pull request Feb 11, 2022
astrovsky01 pushed a commit to astrovsky01/galaxy that referenced this pull request Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants