Skip to content

[HIVE-21686] ensure that memory allocator does not evict using brute …#626

Closed
b-slim wants to merge 6 commits intoapache:masterfrom
b-slim:HIVE-21686
Closed

[HIVE-21686] ensure that memory allocator does not evict using brute …#626
b-slim wants to merge 6 commits intoapache:masterfrom
b-slim:HIVE-21686

Conversation

@b-slim
Copy link
Contributor

@b-slim b-slim commented May 13, 2019

[HIVE-21686] ensure that memory allocator does not evict using brute foce path.

Change-Id: I020d9be9e76ac095f909b53cbec9e63f21efe8ff

…foce path.

Change-Id: I020d9be9e76ac095f909b53cbec9e63f21efe8ff
Copy link
Contributor

@odraese odraese left a comment

Choose a reason for hiding this comment

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

Mainly minor stuff, except the one portion of the
allocLog2 << dest.length - destAllocIx + forceEvictAttempt - MAX_FAST_ATTEMPT
which I either don't understand or which is wrong....

* @param memoryToReserve amount of bytes to evict
* @return actual amount of evicted bytes.
*/
long evictMemory(long memoryToReserve);
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding this to MemoryManager kind'o breaks the separation of memory manager and eviction policy (at least from interface perspective, these were two separate things so far).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Memory manager is doing eviction on reserve call, thus this is adding an explicit handle to the eviction without reserving it. do you a different idea?

b-slim added 5 commits May 13, 2019 21:10
Change-Id: I5664256e4350536546cdee9a92fb7a39df2c9980
Change-Id: I1ffc034b98f5f008d0b54fd1b6006dbbb7b5b4ad
Change-Id: Ibeacf2a29cc6bde452a64b548c4fa49d4a4ce870
Change-Id: I7ab1279a3375c3cf2f36bf75dc798cb35bb91a9f
Change-Id: I7a14811c2c04e95f82ccd58b162ea38b398691d9
@b-slim
Copy link
Contributor Author

b-slim commented May 14, 2019

@odraese Thanks for the review, i did fix most of your comments can you please take a second look when you have time.

Copy link
Contributor

@odraese odraese left a comment

Choose a reason for hiding this comment

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

Looking good. Thx for the changes!

@b-slim
Copy link
Contributor Author

b-slim commented May 15, 2019

thanks pushed to master

@b-slim b-slim closed this May 15, 2019
@b-slim b-slim deleted the HIVE-21686 branch March 7, 2020 01:04
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