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

[CI] Extract interpreter workflow and split std_spec execution #13267

Merged
merged 2 commits into from
Apr 4, 2023

Conversation

straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented Apr 1, 2023

test_interpreter is failing almost constantly in CI due to running out of memory.

Execution of interpreter-std_spec uses almost 10G memory, but the GHA runners only have around 8G available.

        Command being timed: "bin/crystal i spec/interpreter_std_spec.cr -- --junit_output .junit/interpreter-std_spec.xml"
        User time (seconds): 369.31
        System time (seconds): 4.90
        Percent of CPU this job got: 277%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 2:15.03
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 9926492
        Average resident set size (kbytes): 0
        Major (requiring I/O) page faults: 0
        Minor (reclaiming a frame) page faults: 686100
        Voluntary context switches: 21253
        Involuntary context switches: 6934
        Swaps: 0
        File system inputs: 0
        File system outputs: 6128
        Socket messages sent: 0
        Socket messages received: 0
        Signals delivered: 0
        Page size (bytes): 4096
        Exit status: 0

Splitting the entire spec suite into separate pieces reduces the total amount of memory. We already have the mechanism with SPEC_SPLIT available. It was previously used for CI on x86.

We should probably try to reduce the interpreters memory usage, but this is a quick workaround to get CI green again.

Copy link
Contributor

@Sija Sija left a comment

Choose a reason for hiding this comment

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

Several naming suggestions

@@ -0,0 +1,64 @@
name: Interpreter Test
Copy link
Contributor

Choose a reason for hiding this comment

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

Naming suggestion to keep consistency with the rest of the workflows.

Suggested change
name: Interpreter Test
name: Interpreter CI

SPEC_SPLIT_DOTS: 160

jobs:
test-interpreter_spec:
Copy link
Contributor

Choose a reason for hiding this comment

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

Naming consistency-wise:

Suggested change
test-interpreter_spec:
test-interpreter-spec:

Copy link
Member Author

Choose a reason for hiding this comment

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

The spec suite that's being run is called interpreter_spec.

runs-on: ubuntu-22.04
container:
image: crystallang/crystal:1.7.3-build
name: "Test Interpreter"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd avoid redundant naming (workflow name points at the Interpreter already)

Suggested change
name: "Test Interpreter"
name: Test

runs-on: ubuntu-22.04
container:
image: crystallang/crystal:1.7.3-build
name: Build interpreter
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Suggested change
name: Build interpreter
name: Build

Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

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

LGTM

@beta-ziliani beta-ziliani added this to the 1.8.0 milestone Apr 3, 2023
@straight-shoota straight-shoota merged commit 58ec6c2 into master Apr 4, 2023
@straight-shoota straight-shoota deleted the feature/ci-interpreter branch April 4, 2023 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants