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

Undo Functionality for Attribute Renames / Deletion + New Implementation of Committing Changing to Disk #239

Merged
merged 27 commits into from Dec 25, 2021

Conversation

lapp0
Copy link
Collaborator

@lapp0 lapp0 commented Dec 9, 2021

fixes #72

fixes #57

  • implement undo via Updates
  • parser.persist_updates: iterate over Updates and apply their changes
    • Possible better alternative: update configuration files by calculating changes to OptionTree
  • ensure DiffWidget works with the changes
  • update architecture.org to reflect how Updates and persisting now work in Nix-Gui
  • handle all TODOs
  • (referenced / created separate issues below)
  • update version to nix-gui to 0.2.0, as this fixes the last remaining issue in milestone 1 (moved to New v0.2.0 Screenshot #240)

New issues to address shortcomings of this PR

During creation of this PR, new (non-urgent) requirements for Nix-Gui have been discovered / created, but are out of scope for this already-oversized PR.

Immediate follow up PR

Code injection is messy and doesn't work properly at all for lists.

@lapp0 lapp0 changed the title [WIP] Revert and Undo for [WIP] Revert / Undo + New Implementation of Committing Changing to Disk Dec 10, 2021
@lapp0 lapp0 changed the title [WIP] Revert / Undo + New Implementation of Committing Changing to Disk [WIP] Undo Renames and Attribute Deletion + New Implementation of Committing Changing to Disk Dec 10, 2021
@lapp0 lapp0 changed the title [WIP] Undo Renames and Attribute Deletion + New Implementation of Committing Changing to Disk [WIP] Undo RFunctionality for enames and Attribute Deletion + New Implementation of Committing Changing to Disk Dec 10, 2021
@lapp0 lapp0 changed the title [WIP] Undo RFunctionality for enames and Attribute Deletion + New Implementation of Committing Changing to Disk [WIP] Undo Functionality for Attribute Renames / Deletion + New Implementation of Committing Changing to Disk Dec 10, 2021
@lapp0
Copy link
Collaborator Author

lapp0 commented Dec 17, 2021

Attempting to cache OptionTree functions based on changes to in_memory_cache. This is more expensive to calculate than changing then checking the change_marker. However, it gives us the ability to reuse old cached results if the in_memory_cache is reverted. Additionally, it is a more coherent model.

Testing results for ``tests/test_option_tree.py::test_lru_cache_fast` below.

        1    0.010    0.010 3892.400 3892.400 test_option_tree.py:57(test_lru_cache_fast)
    14394    2.877    0.000 3862.389    0.268 option_tree.py:81(__hash__)
  14355/1    0.104    0.000 3852.905 3852.905 option_tree.py:298(count_leaves)
   2501/1    0.232    0.000 3852.905 3852.905 {built-in method builtins.sum}
    14454   89.177    0.006 3307.828    0.229 {built-in method builtins.sorted}
 32817590  238.372    0.000 3141.048    0.000 attribute.py:51(__lt__)
 65646319  215.903    0.000 2711.201    0.000 attribute.py:55(__str__)
 65646319  537.903    0.000 2435.324    0.000 attribute.py:61(<listcomp>)
328708610  573.728    0.000 1897.424    0.000 re.py:188(match)
328708917  566.456    0.000  852.991    0.000 re.py:289(_compile)
44009963/312612   60.418    0.000  552.338    0.002 {built-in method builtins.hash}
 14565720   68.506    0.000  481.796    0.000 option_definition.py:162(__hash__)
328715305  470.825    0.000  470.825    0.000 {method 'match' of 're.Pattern' objects}
 14565720   47.334    0.000  392.160    0.000 hash_by_json.py:13(hash_object)
 14565723   96.365    0.000  337.442    0.000 __init__.py:183(dumps)
358057957/358057948  303.642    0.000  303.692    0.000 {built-in method builtins.isinstance}
 14565723   74.518    0.000  204.265    0.000 encoder.py:182(encode)
131286801/65650743  126.217    0.000  191.885    0.000 {built-in method builtins.len}
 14565723  106.192    0.000  106.192    0.000 encoder.py:204(iterencode)
 65635332   65.667    0.000   96.244    0.000 attribute.py:48(__len__)
80235306/80234700   66.496    0.000   66.506    0.000 {method 'join' of 'str' objects}
 14565724   36.812    0.000   36.812    0.000 encoder.py:104(__init__)
 14860492   25.557    0.000   33.428    0.000 attribute.py:71(__hash__)
     11/1    0.013    0.001   28.592   28.592 cache.py:62(wrapper)
        1    0.000    0.000   28.586   28.586 api.py:10(get_option_tree)
        8    0.001    0.000   22.377    2.797 nix_eval.py:44(nix_instantiate_eval)

Observations:

  • __hash__ takes a quarter second on average, not good
  • This test was done with many Undefined, which are quick and easy to hash, so the cost of option_definition.py:162(__hash__) may be underreported
  • Attribute.__lt__ for sorting is an expensive operation because it calls a re.match many times on an uncompiled regexp.

Solution: compile regexp, lru_cache regexp result.

@lapp0
Copy link
Collaborator Author

lapp0 commented Dec 18, 2021

Improved result

        1    0.010    0.010 1652.020 1652.020 test_option_tree.py:57(test_lru_cache_fast)
    14465    2.824    0.000 1623.365    0.112 option_tree.py:81(__hash__)
  14355/1    0.101    0.000 1614.034 1614.034 option_tree.py:298(count_leaves)
   2501/1    0.213    0.000 1614.034 1614.034 {built-in method builtins.sum}
    14525   76.293    0.005 1066.960    0.073 {built-in method builtins.sorted}
 32979279  188.776    0.000  917.820    0.000 attribute.py:53(__lt__)
 65969696  197.845    0.000  571.359    0.000 attribute.py:57(__str__)
44225657/312750   59.890    0.000  553.942    0.002 {built-in method builtins.hash}
 14637572   69.997    0.000  484.900    0.000 option_definition.py:162(__hash__)
 14637572   46.429    0.000  393.826    0.000 hash_by_json.py:13(hash_object)
 14637575   97.900    0.000  339.659    0.000 __init__.py:183(dumps)
 65969696  329.374    0.000  329.398    0.000 attribute.py:58(<listcomp>)
 14637575   76.085    0.000  205.499    0.000 encoder.py:182(encode)
131933486/65974050  100.314    0.000  157.819    0.000 {built-in method builtins.len}
 14637575  105.933    0.000  105.933    0.000 encoder.py:204(iterencode)
 65958710   57.504    0.000   80.979    0.000 attribute.py:50(__len__)
80630182/80629576   50.198    0.000   50.207    0.000 {method 'join' of 'str' objects}
 14637576   36.260    0.000   36.260    0.000 encoder.py:104(__init__)
 14932415   24.803    0.000   32.550    0.000 attribute.py:68(__hash__)
     11/1    0.009    0.001   27.314   27.314 cache.py:62(wrapper)
        1    0.000    0.000   27.309   27.309 api.py:10(get_option_tree)
        8    0.001    0.000   21.572    2.697 nix_eval.py:44(nix_instantiate_eval)
        8    0.001    0.000   21.434    2.679 nix_eval.py:33(nix_instantiate)
        8    0.001    0.000   21.419    2.677 subprocess.py:464(run)
        8    0.000    0.000   21.341    2.668 subprocess.py:1090(communicate)
        8    0.020    0.003   21.341    2.668 subprocess.py:1926(_communicate)

Seems sorting is taking a while, I'll try SortedDict

@lapp0
Copy link
Collaborator Author

lapp0 commented Dec 18, 2021

Substantially improved by implementing CachedHashDict

        1    2.647    2.647   66.963   66.963 test_option_tree.py:57(test_lru_cache_fast)
     1012    0.005    0.000   37.886    0.037 cached_hash_dict.py:20(__setitem__)
     1013    0.055    0.000   37.879    0.037 cached_hash_dict.py:37(_recalculate_hash)
     1000    0.009    0.000   37.050    0.037 option_tree.py:197(insert_attribute)
     1073    1.722    0.002   25.465    0.024 {built-in method builtins.sorted}
  1127360    3.998    0.000   20.060    0.000 attribute.py:53(__lt__)
     11/1    0.007    0.001   17.908   17.908 cache.py:62(wrapper)
        1    0.000    0.000   17.904   17.904 api.py:10(get_option_tree)
        8    0.001    0.000   14.329    1.791 nix_eval.py:44(nix_instantiate_eval)
        8    0.001    0.000   14.246    1.781 nix_eval.py:33(nix_instantiate)
        8    0.001    0.000   14.234    1.779 subprocess.py:464(run)
        8    0.000    0.000   14.165    1.771 subprocess.py:1090(communicate)
        8    0.015    0.002   14.165    1.771 subprocess.py:1926(_communicate)
      794    0.012    0.000   14.126    0.018 selectors.py:403(select)
      794   14.112    0.018   14.112    0.018 {method 'poll' of 'select.poll' objects}
3309238/1771301    2.424    0.000   13.803    0.000 {built-in method builtins.hash}
  2265859    4.330    0.000   12.692    0.000 attribute.py:57(__str__)
   512582    1.625    0.000   10.919    0.000 option_definition.py:162(__hash__)
   512582    1.100    0.000    8.773    0.000 hash_by_json.py:13(hash_object)
        1    0.001    0.001    8.130    8.130 nix_eval.py:82(get_all_nixos_options)
   512585    2.120    0.000    7.514    0.000 __init__.py:183(dumps)
  2265859    7.396    0.000    7.418    0.000 attribute.py:58(<listcomp>)
   546293    2.085    0.000    4.898    0.000 attribute.py:12(__init__)
   512585    1.719    0.000    4.580    0.000 encoder.py:182(encode)
      3/1    0.001    0.000    4.452    4.452 parser.py:59(get_all_option_values)
4525553/2269234    2.163    0.000    3.462    0.000 {built-in method builtins.len}

@lapp0
Copy link
Collaborator Author

lapp0 commented Dec 18, 2021

Massively improved with improvements to CachedHashDict:

584459 0.454 0.000 1.638 0.000 option_tree.py:81(__hash__)

Copy link
Collaborator Author

@lapp0 lapp0 left a comment

Choose a reason for hiding this comment

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

A few changes, still need architecture.org updated as well

nixui/options/attribute.py Outdated Show resolved Hide resolved
nixui/options/option_tree.py Outdated Show resolved Hide resolved
@lapp0 lapp0 changed the title [WIP] Undo Functionality for Attribute Renames / Deletion + New Implementation of Committing Changing to Disk Undo Functionality for Attribute Renames / Deletion + New Implementation of Committing Changing to Disk Dec 20, 2021
@lapp0 lapp0 mentioned this pull request Dec 20, 2021
4 tasks
@lapp0 lapp0 merged commit b1b1e64 into master Dec 25, 2021
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.

DynamicAttrsOf Improvements Ability to delete an option from configuration
1 participant