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

Lazy read of attributes #1026

Merged
merged 19 commits into from
Jun 26, 2018

Conversation

edhartnett
Copy link
Contributor

@edhartnett edhartnett commented Jun 21, 2018

Part of #857.
Part of #489.
Fixes #1025.

In this PR I change attribute reading to lazy. No attributes are read at open. Whenever the user gets/puts/inqs an attribute for a var or group, then all atts for that var or group are read. If atts in a var/group are never touched by the user, they are never read.

This helps a lot with slow open times for files with many attributes.

@edhartnett edhartnett closed this Jun 21, 2018
@edhartnett edhartnett reopened this Jun 22, 2018
@edhartnett edhartnett closed this Jun 22, 2018
@edhartnett edhartnett reopened this Jun 22, 2018
@edhartnett edhartnett closed this Jun 22, 2018
@DennisHeimbigner
Copy link
Collaborator

Did this get merged? Why is it closed?

@edhartnett
Copy link
Contributor Author

This is in the queue. When #1023 gets merged this will be next.

@edhartnett edhartnett reopened this Jun 22, 2018
Copy link
Collaborator

@DennisHeimbigner DennisHeimbigner left a comment

Choose a reason for hiding this comment

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

nc4_set_default_vars needs to be restored

@edhartnett
Copy link
Contributor Author

nc4_set_default_vars does not have a place any more. It used to call the vara versions of the function. But now there is no vara version of the function. The vara calls also call the vars functions.

So to see the difference between breaking up the write/read into varas instead of vars, you can call NCDEFAULT_vars, but it will still end up calling the vars functions under the hood.

@DennisHeimbigner
Copy link
Collaborator

Then I will put back NC4_get/put_vara and NC4_get/put_vars

@edhartnett
Copy link
Contributor Author

OK, you may wish to simply unwind the most recent PR that was merged, which included those changes.

Be advised that although I didn't enter tickets for it, I believe there were some additional issues in that version of the vars functions. Handling of zero counts did not work until I made some changes to the vars code. I suspect there are simply more and better tests for the vara code.

I will take down this PR while all this is resolved.

@edhartnett edhartnett closed this Jun 23, 2018
@edhartnett edhartnett reopened this Jun 25, 2018
@edhartnett
Copy link
Contributor Author

OK, Dennis and I have resolved the request for changes, and this PR is ready to merge.

@WardF
Copy link
Member

WardF commented Jun 25, 2018

@DennisHeimbigner can you remove your request for changes? Thanks!

@WardF WardF merged commit b9d8468 into Unidata:master Jun 26, 2018
@edhartnett edhartnett deleted the ejh_lazy_atts_2 branch June 26, 2018 08:29
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.

3 participants