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

Memorize the C string representation. #495

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

cjgillot
Copy link

@cjgillot cjgillot commented Jun 7, 2016

For some cases, the computation of the C
string for inline using _getRepresentation
takes a substantial amount of time.

Here, we memorize the computed C code and argument dictionary.
The argDict used to store the values of the required variables.
Now it stores the variables themselves,
and the values are retrieved by _execInline.

I have been using this patch offline for a while,
and it does not seem to harm anything.
What is your opinion about this ?

For some cases, the computation of the C
string for inline using _getRepresentation
takes a substantial amount of time.

Here, we memorize the computed C code and argument dictionary.
The argDict used to store the values of the required variables.
Now it stores the variables themselves,
and the values are retrieved by _execInline.
@wd15
Copy link
Contributor

wd15 commented Aug 1, 2016

@cjgillot, thanks for this, we currently don't have the time to look into this pull request, but in the next round of FiPy development we will likely merge try and merge it.

@tkphd tkphd requested review from guyer, wd15 and tkphd January 3, 2018 19:43
@wd15
Copy link
Contributor

wd15 commented Jan 4, 2018

This might be too difficult for me to get into right now. I think I'll take myself off this.

@wd15 wd15 removed their request for review January 4, 2018 15:42
@guyer guyer added this to the 3.2 milestone Jun 27, 2018
@guyer guyer removed this from the 3.2 milestone Aug 31, 2018
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.

None yet

3 participants