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

[R] Move gc data protection to R side #11104

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

david-cortes
Copy link
Contributor

ref #9810
ref #11092 (comment)

This PR moves the logic for protection of R data from the garbage collector towards the R side.

I'm not sure whether data holding fields of the DMatrix also needs to be kept protected after setting it in the proxy dmatrix, but this PR does so just in case.

@trivialfis
Copy link
Member

Let me try to create a prototype for the #11088 and see how this should work. I need to store the categories somewhere.

@trivialfis
Copy link
Member

Should we revert #11092 before applying this PR? The previous one seems redundant now.

@david-cortes
Copy link
Contributor Author

Should we revert #11092 before applying this PR? The previous one seems redundant now.

It is undoing the changes from it already.

But it leaves me wondering: what's the difference between xgb.ExtMemDMatrix and xgb.QuantileDMatrix.from_iterator? Both of them seem to apply exclusively to histogram-based models, and both of them seem to access the data the same amount of times.

Is xgb.ExtMemDMatrix perhaps meant to leave the iterator alive throughout the call to xgb.train?

@trivialfis
Copy link
Member

The xgb.ExtMemDMatrix stores a cache on disk, whereas the QDM.from_iterator concatenates everything in memory and saves memory by using the compressed histogram bin index instead of raw input.

In addition, the former applies to both approx and hist, while the latter is hist only.

https://github.com/dmlc/xgboost/blob/master/doc/tutorials/external_memory.rst#compared-to-the-quantiledmatrix

@trivialfis
Copy link
Member

I'm not sure whether data holding fields of the DMatrix

If you meant things like labels and weights, no, they don't need any protection.

@david-cortes
Copy link
Contributor Author

I'm not sure whether data holding fields of the DMatrix

If you meant things like labels and weights, no, they don't need any protection.

Thanks, modified the PR to not keep those protected after setting them.

@trivialfis
Copy link
Member

Will take a deeper look tomorrow.

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