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

Increase "local" space of ParamValue #1812

Merged
merged 1 commit into from
Nov 27, 2017

Conversation

lgritz
Copy link
Collaborator

@lgritz lgritz commented Nov 25, 2017

ParamValue holds "small" data locally (in the struct), and only
allocates when above that threshold. Previously, the threshold was 8
bytes, the size that the allcoated pointer would be anyway. But it seems
to me that the way we us ParamValue frequently must contain a color (3
floats), so I bumped the local space from 8 to 16 bytes (meaning it can
hold up to 4 floats or ints before a malloc). This increases the total
size of a ParamValue from 32 to 40 bytes, but I think it will make it
fairly rare to allocate, so we'll come out ahead.

This should all be inconsequential from an API perspective.

ParamValue holds "small" data locally (in the struct), and only
allocates when above that threshold. Previously, the threshold was 8
bytes, the size that the allcoated pointer would be anyway. But it seems
to me that the way we us ParamValue frequently must contain a color (3
floats), so I bumped the local space from 8 to 16 bytes (meaning it can
hold up to 4 floats or ints before a malloc). This increases the total
size of a ParamValue from 32 to 40 bytes, but I think it will make it
fairly rare to allocate, so we'll come out ahead.

This should all be inconsequential from an API perspective.
@fpsunflower
Copy link
Contributor

LGTM

@lgritz lgritz merged commit 1c6a17d into AcademySoftwareFoundation:master Nov 27, 2017
@lgritz lgritz deleted the lg-paramval branch November 27, 2017 17:28
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.

2 participants