Skip to content

Update render queue limits #152

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.

Sign up for GitHub

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

Closed
Komzpa wants to merge 4 commits into from
+5 −9
Closed

Update render queue limits #152

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b0ae60f
Update render queue limits
Komzpa Jan 8, 2017
77d23f7
Fixed review notices
Komzpa Jan 8, 2017
6deaa83
Make hash index linearly larger
Komzpa Nov 16, 2017
bf4a7ca
remove hash index size redeclaration
Komzpa Nov 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions includes/render_config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,18 @@
//Legacy - not needed on new installs
//#undef METATILEFALLBACK

// Metatiles are much larger in size so we don't need big queues to handle large areas
// Typical osm.org render server can render 7 metatiles per second, and has average load of 4.5 metatiles per second.
// Queue depth of (7 - 4.5) * 86400 = 216000 moves the load from the busy part of the day to the night time
#ifdef METATILE
#define QUEUE_MAX (64)
#define REQ_LIMIT (32)
#define DIRTY_LIMIT (1000)

#define DIRTY_LIMIT (216000)
#define HASHIDX_SIZE (477856)
Copy link
Copy Markdown
Contributor

@pnorman pnorman Jan 8, 2017

Choose a reason for hiding this comment

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

Why is a higher HASHIDX_SIZE needed for with metatiles?

Copy link
Copy Markdown
Author

@Komzpa Komzpa Jan 8, 2017

Choose a reason for hiding this comment

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

According to @gardster, this is not required and is redefined anyway later. I did it by analogy to the below. It looks like it's safe to remove both.

#else
#define QUEUE_MAX (1024)
#define REQ_LIMIT (512)
#define DIRTY_LIMIT (10000)
Copy link
Copy Markdown
Contributor

@pnorman pnorman Jan 8, 2017

Choose a reason for hiding this comment

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

Shouldn't the non-metatile limit be higher than the metatile limit?

#define HASHIDX_SIZE 22123
Copy link
Copy Markdown
Contributor

@Zverik Zverik Nov 15, 2017

Choose a reason for hiding this comment

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

What is HASHIDX_SIZE and why are you removing it?

Copy link
Copy Markdown
Author

@Komzpa Komzpa Nov 15, 2017

Choose a reason for hiding this comment

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

This line is no-op.
It is used here:

mod_tile/src/request_queue.c

Line 39 in 62c4a5a

return key % queue->hashidxSize;

and redefined here:

mod_tile/includes/request_queue.h

Line 31 in 62c4a5a

#define HASHIDX_SIZE 2213

(see includes order in

mod_tile/src/request_queue.c

Line 29 in 62c4a5a

#include "render_config.h"
)

Copy link
Copy Markdown
Member

@tomhughes tomhughes Nov 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

So does the hash table have 2213 buckets? or 22123 buckets? Not that either is anywhere big enough once you have a two million entry queue - that will give an average chain length of either 100 or 1000 either of which seems way too large.

Copy link
Copy Markdown

@vng vng Nov 15, 2017

Choose a reason for hiding this comment

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

Seems like somebody wants to increase HASHIDX_SIZE from default 2213 to 22123 when METATILE is not defined. But it's so ugly and include order dependent that should be fixed.

Copy link
Copy Markdown

@vng vng Nov 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@apmon According to the code history, here is the initial commit to use different values for HASHIDX_SIZE:
da19811

But later after the refactoring, it was broken, and now HASHIDX_SIZE is always (not obvious) is 2213:
707e125

Copy link
Copy Markdown
Author

@Komzpa Komzpa Nov 16, 2017

Choose a reason for hiding this comment

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

@tomhughes I've updated numbers to match ratio.
We're not using non-metatile mode, so it's not 2M, but 200k records.

@vng I've updated request_queue.h to not rewrite the value.

#define HASHIDX_SIZE (22123)
#endif

// Penalty for client making an invalid request (in seconds)
Expand Down