-
Notifications
You must be signed in to change notification settings - Fork 15
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
when to use std::int32_t and std::int64_t #98
Comments
cc @mapbox/core-tech |
Here is my thinking around this: Generally it is best to use specific types (like The exception for this is code that needs to interface with C libraries for instance that use the classical types, take extra care in those cases to switch between types. Then there is the question of the size you want. From a data modelling standpoint you want the type that best fits the bill. OSM coordinates or tile coordinates fit in 32 bit because of the limited resolution, but generally coordinates need a double or 64 bit integer. Also smaller is better, because it takes less space in memory and might be faster. This ist most important if there is a lot of the data, say an array of thousands of coordinates. From a CPU performance point of view anything smaller than 32bit is probably not worth it, because registers are 32 or 64 bit anyway, but it still might help with memory/cache performance. signed vs. unsigned: I think it is general wisdom these days that using signed ints is better than unsigned (see http://soundsoftware.ac.uk/c-pitfall-unsigned.html for instance). I have heard several C++ ISO committee members arguing that making There are some goatches with 8 bit types: Classic types are Consistency is really important: Casting between types is always a point where things could break, so minimizing types and type switches is good. So there is no perfect solution here. One other thing I like to think of when deciding is: What happens if something overflows, is this actually a problem? And: What happens if I check for an overflow (say on a downcast) and I get a problem? How can I handle the error or report it? When I don't have a good way of handling this, I must use a larger type. |
Description
Looks like there have been some conversations around this and thought we should discuss all the places where we need to be explicit about the bit size of the int and when we shouldn't. I'll start us off with refs to previous conversations and lists of our current
int32_t
andint64_t
usage. It would be great to get some follow-up comments about the reasoning behind this.It looks like the idea is that we want to use
int32_t
forx
,y
,z
, andextent
until we cast them todouble
s orint64_t
s when we store them in amapbox::geometry::point
. Looks like we useint64_t
forid
and property values, referred to asv
in the code.References
int32_t/ uint32_t usage
create_query_point
- https://github.com/mapbox/vtquery/blob/master/src/util.hpp#L54-L57convert_vt_to_ll
- https://github.com/mapbox/vtquery/blob/master/src/util.hpp#L84-L87TileObject
- https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L64-L66 & https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L93-L95QueryData
- https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L102 & https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L128Execute
- https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L278 & https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L301-L304HandleOKCallback
- https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L433NAN_METHOD(vtquery)
- https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L507-L536 & https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L605-L613 & https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L651int64_t/ uint64_t usage
mapbox::util::variant
- https://github.com/mapbox/vtquery/blob/master/src/util.hpp#L40create_query_point
- https://github.com/mapbox/vtquery/blob/master/src/util.hpp#L52 & https://github.com/mapbox/vtquery/blob/master/src/util.hpp#L68-L78convert_vt_to_ll
- https://github.com/mapbox/vtquery/blob/master/src/util.hpp#L89ResultObject
- https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L44property_value_visitor
- https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L144-L147insert_result
- https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L201Execute
- https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L306 & https://github.com/mapbox/vtquery/blob/master/src/vtquery.cpp#L317The text was updated successfully, but these errors were encountered: