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

Unnecessary to check levelnumber = = 0 in Level.java #118

Open
hecenjie opened this issue Feb 22, 2020 · 3 comments
Open

Unnecessary to check levelnumber = = 0 in Level.java #118

hecenjie opened this issue Feb 22, 2020 · 3 comments

Comments

@hecenjie
Copy link

hecenjie commented Feb 22, 2020

// Version.java
    public LookupResult get(LookupKey key)
    {
        LookupResult lookupResult = level0.get(key, readStats);
        if (lookupResult == null) {
            for (Level level : levels) {
                lookupResult = level.get(key, readStats);
                ...
            }
        }
        ...
        return lookupResult;
    }

// Level0.java
    public LookupResult get(LookupKey key, ReadStats readStats)
    {
        ...
        List<FileMetaData> fileMetaDataList = new ArrayList<>(files.size());
        for (FileMetaData fileMetaData : files) {
            if (...) {
                fileMetaDataList.add(fileMetaData);
            }
        }
        Collections.sort(fileMetaDataList, NEWEST_FIRST);
        ...
    }

// Level.java
    public LookupResult get(LookupKey key, ReadStats readStats)
    {
        ...
        List<FileMetaData> fileMetaDataList = new ArrayList<>(files.size());
        if (levelNumber == 0) {    // Do we really need to judge this condition ?
            for (FileMetaData fileMetaData : files) {
                if (...) {
                    fileMetaDataList.add(fileMetaData);
                }
            }
        }
        else {
            // Binary search to find earliest index whose largest key >= ikey.
        }
        ...
    }

In the above function, level0.get() and level.get() are called respectively. However, In the implementation of the former, calls the sort function Collections.sort(fileMetaDataList, NEWEST_FIRST); to make the newer files in front of the list while the other call doesn't. So do we need this sort function call or not? And why get function in Level.java should consider the situation of level0?

@dain
Copy link
Owner

dain commented Feb 22, 2020

Level 0 files are not compacted, so you have to process them in temporal order to ensure that old (deleted or over written) values are not returned. Later levels are compacted, so you don't need this.

@hecenjie
Copy link
Author

hecenjie commented Feb 23, 2020

I understand that. I mean, does the get method in Level.java need to check the first if-statement (levelnumber = = 0), I think it has been processed by the get method in Level0.java, which also means that the get method in Level.java doesn't need the sort as you said.

@hecenjie
Copy link
Author

In other words, can we just delete the code fragment below in Level.java because this situation has been considered in Level0.java:

        if (levelNumber == 0) {    // Do we really need to judge this condition ?
            for (FileMetaData fileMetaData : files) {
                if (...) {
                    fileMetaDataList.add(fileMetaData);
                }
            }
        }

@hecenjie hecenjie changed the title The necessity of sort function in get(LookupKey key, ReadStats readStats) Unnecessary to check levelnumber = = 0 in Level.java Feb 25, 2020
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

No branches or pull requests

2 participants