Skip to content

Commit 6421329

Browse files
committed
Fixing performance regression caused by helpful code contributions
that moved inlineable functions into the source file combined with helpful compilers which aren't smart enough to do the inlinining in any case.
1 parent 4e7e7d9 commit 6421329

3 files changed

Lines changed: 93 additions & 117 deletions

File tree

benchmark/distinctuseridcompetition.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ void simdjson_traverse(std::vector<int64_t> &answer, ParsedJson::iterator &i) {
3535
case '{':
3636
if (i.down()) {
3737
do {
38-
bool founduser = equals(i.get_string(), "user");
38+
bool founduser = (i.get_string_length() == 4) && (memcmp(i.get_string(), "user", 4) == 0);
3939
i.next(); // move to value
4040
if (i.is_object()) {
4141
if (founduser && i.move_to_key("id")) {
@@ -97,8 +97,11 @@ void sajson_traverse(std::vector<int64_t> &answer, const sajson::value &node) {
9797
}
9898
case TYPE_OBJECT: {
9999
auto length = node.get_length();
100+
// sajson has O(log n) find_object_key, but we still visit each node anyhow
100101
for (auto i = 0u; i < length; ++i) {
101-
if (equals(node.get_object_key(i).data(), "user")) { // found a user!!!
102+
auto key = node.get_object_key(i); // expected: sajson::string
103+
bool founduser = (key.length() == 4) && (memcmp(key.data(), "user", 4) == 0);
104+
if (founduser) { // found a user!!!
102105
auto uservalue = node.get_object_value(i); // get the value
103106
if (uservalue.get_type() ==
104107
TYPE_OBJECT) { // the value should be an object
@@ -153,7 +156,8 @@ void rapid_traverse(std::vector<int64_t> &answer, const rapidjson::Value &v) {
153156
case kObjectType:
154157
for (Value::ConstMemberIterator m = v.MemberBegin(); m != v.MemberEnd();
155158
++m) {
156-
if (equals(m->name.GetString(), "user")) {
159+
bool founduser = (m->name.GetStringLength() == 4) && (memcmp(m->name.GetString(), "user", 4) == 0);
160+
if (founduser) {
157161
const rapidjson::Value &child = m->value;
158162
if (child.GetType() == kObjectType) {
159163
for (Value::ConstMemberIterator k = child.MemberBegin();

include/simdjson/parsedjson.h

Lines changed: 57 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -124,43 +124,85 @@ struct ParsedJson {
124124

125125
// retrieve the character code of what we're looking at:
126126
// [{"sltfn are the possibilities
127-
uint8_t get_type() const;
127+
inline uint8_t get_type() const {
128+
return current_type; // short functions should be inlined!
129+
}
128130

129131
// get the int64_t value at this node; valid only if we're at "l"
130-
int64_t get_integer() const;
132+
inline int64_t get_integer() const {
133+
if(location + 1 >= tape_length) {
134+
return 0;// default value in case of error
135+
}
136+
return static_cast<int64_t>(pj.tape[location + 1]);
137+
}
131138

132139
// get the string value at this node (NULL ended); valid only if we're at "
133140
// note that tabs, and line endings are escaped in the returned value (see print_with_escapes)
134141
// return value is valid UTF-8
135142
// It may contain NULL chars within the string: get_string_length determines the true
136143
// string length.
137-
const char * get_string() const;
144+
inline const char * get_string() const {
145+
return reinterpret_cast<const char *>(pj.string_buf + (current_val & JSONVALUEMASK) + sizeof(uint32_t)) ;
146+
}
138147

139-
uint32_t get_string_length() const;
148+
// return the length of the string in bytes
149+
inline uint32_t get_string_length() const {
150+
uint32_t answer;
151+
memcpy(&answer, reinterpret_cast<const char *>(pj.string_buf + (current_val & JSONVALUEMASK)), sizeof(uint32_t));
152+
return answer;
153+
}
140154

141155
// get the double value at this node; valid only if
142156
// we're at "d"
143-
double get_double() const;
157+
inline double get_double() const {
158+
if(location + 1 >= tape_length) {
159+
return NAN;// default value in case of error
160+
}
161+
double answer;
162+
memcpy(&answer, & pj.tape[location + 1], sizeof(answer));
163+
return answer;
164+
}
165+
144166

145-
bool is_object_or_array() const;
167+
inline bool is_object_or_array() const {
168+
return is_object() || is_array();
169+
}
146170

147-
bool is_object() const;
171+
inline bool is_object() const {
172+
return get_type() == '{';
173+
}
148174

149-
bool is_array() const;
175+
inline bool is_array() const {
176+
return get_type() == '[';
177+
}
150178

151-
bool is_string() const;
179+
inline bool is_string() const {
180+
return get_type() == '"';
181+
}
152182

153-
bool is_integer() const;
183+
inline bool is_integer() const {
184+
return get_type() == 'l';
185+
}
154186

155-
bool is_double() const;
187+
inline bool is_double() const {
188+
return get_type() == 'd';
189+
}
156190

157-
bool is_true() const;
191+
inline bool is_true() const {
192+
return get_type() == 't';
193+
}
158194

159-
bool is_false() const;
195+
inline bool is_false() const {
196+
return get_type() == 'f';
197+
}
160198

161-
bool is_null() const;
199+
inline bool is_null() const {
200+
return get_type() == 'n';
201+
}
162202

163-
static bool is_object_or_array(uint8_t type);
203+
static bool is_object_or_array(uint8_t type) {
204+
return ((type == '[') || (type == '{'));
205+
}
164206

165207
// when at {, go one level deep, looking for a given key
166208
// if successful, we are left pointing at the value,

src/parsedjsoniterator.cpp

Lines changed: 29 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -105,78 +105,6 @@ bool ParsedJson::iterator::move_forward() {
105105
return true;
106106
}
107107

108-
uint8_t ParsedJson::iterator::get_type() const {
109-
return current_type;
110-
}
111-
112-
113-
int64_t ParsedJson::iterator::get_integer() const {
114-
if(location + 1 >= tape_length) {
115-
return 0;// default value in case of error
116-
}
117-
return static_cast<int64_t>(pj.tape[location + 1]);
118-
}
119-
120-
double ParsedJson::iterator::get_double() const {
121-
if(location + 1 >= tape_length) {
122-
return NAN;// default value in case of error
123-
}
124-
double answer;
125-
memcpy(&answer, & pj.tape[location + 1], sizeof(answer));
126-
return answer;
127-
}
128-
129-
const char * ParsedJson::iterator::get_string() const {
130-
return reinterpret_cast<const char *>(pj.string_buf + (current_val & JSONVALUEMASK) + sizeof(uint32_t)) ;
131-
}
132-
133-
134-
uint32_t ParsedJson::iterator::get_string_length() const {
135-
uint32_t answer;
136-
memcpy(&answer, reinterpret_cast<const char *>(pj.string_buf + (current_val & JSONVALUEMASK)), sizeof(uint32_t));
137-
return answer;
138-
}
139-
140-
bool ParsedJson::iterator::is_object_or_array() const {
141-
return is_object_or_array(get_type());
142-
}
143-
144-
bool ParsedJson::iterator::is_object() const {
145-
return get_type() == '{';
146-
}
147-
148-
bool ParsedJson::iterator::is_array() const {
149-
return get_type() == '[';
150-
}
151-
152-
bool ParsedJson::iterator::is_string() const {
153-
return get_type() == '"';
154-
}
155-
156-
bool ParsedJson::iterator::is_integer() const {
157-
return get_type() == 'l';
158-
}
159-
160-
bool ParsedJson::iterator::is_double() const {
161-
return get_type() == 'd';
162-
}
163-
164-
bool ParsedJson::iterator::is_true() const {
165-
return get_type() == 't';
166-
}
167-
168-
bool ParsedJson::iterator::is_false() const {
169-
return get_type() == 'f';
170-
}
171-
172-
bool ParsedJson::iterator::is_null() const {
173-
return get_type() == 'n';
174-
}
175-
176-
bool ParsedJson::iterator::is_object_or_array(uint8_t type) {
177-
return (type == '[' || (type == '{'));
178-
}
179-
180108
bool ParsedJson::iterator::move_to_key(const char * key) {
181109
if(down()) {
182110
do {
@@ -195,24 +123,25 @@ bool ParsedJson::iterator::move_to_key(const char * key) {
195123

196124
bool ParsedJson::iterator::next() {
197125
if ((current_type == '[') || (current_type == '{')){
198-
// we need to jump
199-
size_t npos = ( current_val & JSONVALUEMASK);
200-
if(npos >= tape_length) {
126+
// we need to jump
127+
size_t npos = ( current_val & JSONVALUEMASK);
128+
if(npos >= tape_length) {
201129
return false; // shoud never happen unless at the root
202-
}
203-
uint64_t nextval = pj.tape[npos];
204-
uint8_t nexttype = (nextval >> 56);
205-
if((nexttype == ']') || (nexttype == '}')) {
130+
}
131+
uint64_t nextval = pj.tape[npos];
132+
uint8_t nexttype = (nextval >> 56);
133+
if((nexttype == ']') || (nexttype == '}')) {
206134
return false; // we reached the end of the scope
207-
}
208-
location = npos;
209-
current_val = nextval;
210-
current_type = nexttype;
211-
return true;
135+
}
136+
location = npos;
137+
current_val = nextval;
138+
current_type = nexttype;
139+
return true;
212140
}
213141
size_t increment = (current_type == 'd' || current_type == 'l') ? 2 : 1;
214-
if(location + increment >= tape_length) { return false;
215-
}
142+
if(location + increment >= tape_length) {
143+
return false;
144+
}
216145
uint64_t nextval = pj.tape[location + increment];
217146
uint8_t nexttype = (nextval >> 56);
218147
if((nexttype == ']') || (nexttype == '}')) {
@@ -222,33 +151,33 @@ bool ParsedJson::iterator::move_to_key(const char * key) {
222151
current_val = nextval;
223152
current_type = nexttype;
224153
return true;
225-
226154
}
227155

228156

229157
bool ParsedJson::iterator::prev() {
230-
if(location - 1 < depthindex[depth].start_of_scope) { return false;
231-
}
158+
if(location - 1 < depthindex[depth].start_of_scope) {
159+
return false;
160+
}
232161
location -= 1;
233162
current_val = pj.tape[location];
234163
current_type = (current_val >> 56);
235164
if ((current_type == ']') || (current_type == '}')){
236-
// we need to jump
237-
size_t new_location = ( current_val & JSONVALUEMASK);
238-
if(new_location < depthindex[depth].start_of_scope) {
165+
// we need to jump
166+
size_t new_location = ( current_val & JSONVALUEMASK);
167+
if(new_location < depthindex[depth].start_of_scope) {
239168
return false; // shoud never happen
240-
}
241-
location = new_location;
242-
current_val = pj.tape[location];
243-
current_type = (current_val >> 56);
169+
}
170+
location = new_location;
171+
current_val = pj.tape[location];
172+
current_type = (current_val >> 56);
244173
}
245174
return true;
246175
}
247176

248177

249178
bool ParsedJson::iterator::up() {
250179
if(depth == 1) {
251-
return false; // don't allow moving back to root
180+
return false; // don't allow moving back to root
252181
}
253182
to_start_scope();
254183
// next we just move to the previous value
@@ -261,8 +190,9 @@ bool ParsedJson::iterator::move_to_key(const char * key) {
261190

262191

263192
bool ParsedJson::iterator::down() {
264-
if(location + 1 >= tape_length) { return false;
265-
}
193+
if(location + 1 >= tape_length) {
194+
return false;
195+
}
266196
if ((current_type == '[') || (current_type == '{')) {
267197
size_t npos = (current_val & JSONVALUEMASK);
268198
if(npos == location + 2) {

0 commit comments

Comments
 (0)