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

Mono: Avoid dictionary lookup for common colors #80047

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

AbeniMatteo
Copy link
Contributor

Accessing statically defined colors from Colors is now an O(1) operation.
-color is also affected since it is implemented as Colors.White - color.

Namespace Method Mean Ratio Code Size
Old GetColorByProp 4,762.49 us 1.00 1,811 B
New GetColorByProp 71.72 us 0.02 46 B
Old InvertColor 2,066.48 us 1.00 1,602 B
New InvertColor 265.14 us 0.13 184 B
public float GetColorByProp()
    var r = 0f;
    for (int i = 0; i < 100_000; i++)
        r += Colors.Red.R + Colors.Black.R + Colors.White.R;
    return r;
}

public float InvertColor()
{
    var color = Colors.Red;
    for (int i = 0; i < 100_000; i++)
        color = -color;
    return color.A;
}

Copy link
Member

@raulsntos raulsntos left a comment

Choose a reason for hiding this comment

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

Looks great, thanks a lot!

@raulsntos raulsntos modified the milestones: 4.x, 4.2 Jul 30, 2023
Copy link
Member

@RedworkDE RedworkDE left a comment

Choose a reason for hiding this comment

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

Doesn't accidentally change the value of any color.

@YuriSizov YuriSizov merged commit 02f04a3 into godotengine:master Aug 1, 2023
@YuriSizov
Copy link
Contributor

YuriSizov commented Aug 1, 2023

Thanks! And congrats on your first merged Godot PR!

@AbeniMatteo AbeniMatteo deleted the dev-colors branch August 2, 2023 08:28
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.

5 participants