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

Change Intercycle offsets from list to set. #2291

Merged
merged 1 commit into from
May 17, 2017

Conversation

oliver-sanders
Copy link
Member

Whilst profiling cylc's memory usage for a suite I noticed that a taskdef was using 7.3Mb. This was largely due to 48'000 items (mostly duplicates) in the taskdef.intercycle_offsets list. This pull reduces this memory usage to 2.9Mb which amounts to roughly a 5% reduction in memory usage for the Scheduler object in this case.

The example in question is a somewhat extreme case, profiling results for the complex suite show a much smaller improvement:

Version Run Elapsed Time (s) CPU Time - Total (s) Max Memory (kb)
intercycle-offsets-to-set complex suite 1064.4 316.7 84456.0
master complex suite 1049.4 317.4 84552.0

@oliver-sanders oliver-sanders added the efficiency For notable efficiency improvements label May 17, 2017
@oliver-sanders oliver-sanders added this to the next release milestone May 17, 2017
@oliver-sanders oliver-sanders self-assigned this May 17, 2017
@matthewrmshin
Copy link
Contributor

As discussed, please add an example suite (to profile-battery?) which demonstrates the extreme example. As this is in taskdef, the memory reduction should show up in cylc validate.

Copy link
Contributor

@matthewrmshin matthewrmshin left a comment

Choose a reason for hiding this comment

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

Change looks good.

@oliver-sanders
Copy link
Member Author

After a bit of fiddling I've come up with a reasonably generic example. The results aren't quite as extreme but demonstrate the problem.

For this suite, looking at the memory usage of the offender taskdef we can see a 300kb reduction in memory usage.

Before

<cylc.taskdef.TaskDef object at 0x3053998> size=3226832 flat=216
    triggers size=2901832 flat=3352
    intercycle_offsets size=309056 flat=21056
    rtconfig size=13432 flat=280
    elapsed_times size=624 flat=624
    ...

After

<cylc.taskdef.TaskDef object at 0x2ca7998> size=2918128 flat=216
    triggers size=2901832 flat=3352
    rtconfig size=13432 flat=280
    elapsed_times size=624 flat=624
    intercycle_offsets size=352 flat=232
    ...

Demonstration Suite

#!Jinja2

[cylc]
    UTC mode = True
[scheduling]
    initial cycle point = 20150615T06
    final cycle point = 20150711T12
    [[dependencies]]
        [[[ T00, T01, T02, T03, T04, T05, T06, T07, T08, T09, T10, T11, T12, T13, T14, T15, T16, T17, T18, T19, T20, T21, T22, T23 ]]]
            graph = """
            {% for I in range( 1, ENSEMBLE_SIZE | int + 1 ) %}
                ensemble_a_{{I}}
                ensemble_a_{{I}}[-PT1H] | offender[-PT1H] => ensemble_b_{{I}}
            {% endfor %}

                FAM_A:succeed-all | offender => dummy_task => FAM_B
                FAM_A:fail-any => offender
                offender => ! FAM_A
                FAM_A:succeed-all => ! offender
            """
[runtime]
    [[root]]
        script = sleep 1

    [[FAM_A, FAM_B]]

    [[offender]]

    {% for I in range(1, ENSEMBLE_SIZE | int + 1) %}
    [[ensemble_a_{{I}}]]
    [[ensemble_b_{{I}}]]
    [[family_a_member_{{I}}]]
        inherit = FAM_A
    [[family_b_member_{{I}}]]
        inherit = FAM_B
    {% endfor %}

Profiling of cylc validate on the original suite shows a 27Mb reduction in memory, sadly this amounts to an overall reduction of only 1.65%.

Version Run Elapsed Time (s) CPU Time - Total (s) Max Memory (kb)
cylc/intercycle-offsets-to-set u-al307 550.8 551.2 1591528.0
master u-al307 562.8 563.1 1618252.0

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

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

Looks good.

@hjoliver hjoliver merged commit 93dd898 into cylc:master May 17, 2017
@oliver-sanders oliver-sanders deleted the intercycle-offsets-to-set branch May 23, 2017 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
efficiency For notable efficiency improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants