Skip to content

Commit 00e9f9d

Browse files
gkorlandgkorland
authored
Fix json.arrappend performance (RedisJSON#324)
* improve arr_pop, arr_insert & arr_trim performance * refactor do_json_* functions
1 parent a9e423d commit 00e9f9d

File tree

3 files changed

+185
-142
lines changed

3 files changed

+185
-142
lines changed

‎src/lib.rs

+144-117
Original file line numberDiff line numberDiff line change
@@ -298,20 +298,23 @@ fn json_bool_toggle(ctx: &Context, args: Vec<String>) -> RedisResult {
298298
.get_value::<RedisJSON>(&REDIS_JSON_TYPE)?
299299
.ok_or_else(RedisError::nonexistent_key)
300300
.and_then(|doc| {
301-
doc.value_op(&path, |value| {
302-
value
303-
.as_bool()
304-
.ok_or_else(|| err_json(value, "boolean"))
305-
.and_then(|curr| {
306-
let result = curr ^ true;
307-
Ok(Value::Bool(result))
308-
})
309-
})
310-
.map(|v| {
311-
ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_toggle", key.as_str());
312-
ctx.replicate_verbatim();
313-
v.to_string().into()
314-
})
301+
doc.value_op(
302+
&path,
303+
|value| {
304+
value
305+
.as_bool()
306+
.ok_or_else(|| err_json(value, "boolean"))
307+
.map(|curr| {
308+
let result = curr ^ true;
309+
Value::Bool(result)
310+
})
311+
},
312+
|result| {
313+
ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_toggle", key.as_str());
314+
ctx.replicate_verbatim();
315+
Ok(result.to_string().into())
316+
},
317+
)
315318
.map_err(|e| e.into())
316319
})
317320
}
@@ -338,14 +341,15 @@ where
338341
.get_value::<RedisJSON>(&REDIS_JSON_TYPE)?
339342
.ok_or_else(RedisError::nonexistent_key)
340343
.and_then(|doc| {
341-
doc.value_op(&path, |value| {
342-
do_json_num_op(&number, value, &op_i64, &op_f64)
343-
})
344-
.map(|v| {
345-
ctx.notify_keyspace_event(NotifyEvent::MODULE, cmd, key.as_str());
346-
ctx.replicate_verbatim();
347-
v.to_string().into()
348-
})
344+
doc.value_op(
345+
&path,
346+
|value| do_json_num_op(&number, value, &op_i64, &op_f64),
347+
|result| {
348+
ctx.notify_keyspace_event(NotifyEvent::MODULE, cmd, key.as_str());
349+
ctx.replicate_verbatim();
350+
Ok(result.to_string().into())
351+
},
352+
)
349353
.map_err(|e| e.into())
350354
})
351355
}
@@ -416,13 +420,19 @@ fn json_str_append(ctx: &Context, args: Vec<String>) -> RedisResult {
416420
.get_value::<RedisJSON>(&REDIS_JSON_TYPE)?
417421
.ok_or_else(RedisError::nonexistent_key)
418422
.and_then(|doc| {
419-
doc.value_op(&path, |value| do_json_str_append(&json, value))
420-
.map(|v| {
423+
doc.value_op(
424+
&path,
425+
|value| do_json_str_append(&json, value),
426+
|result| {
421427
ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_strappend", key.as_str());
422428
ctx.replicate_verbatim();
423-
v.as_str().map_or(usize::MAX, |v| v.len()).into()
424-
})
425-
.map_err(|e| e.into())
429+
Ok(result
430+
.as_str()
431+
.map_or(usize::MAX, |result| result.len())
432+
.into())
433+
},
434+
)
435+
.map_err(|e| e.into())
426436
})
427437
}
428438

@@ -459,31 +469,37 @@ fn json_arr_append(ctx: &Context, args: Vec<String>) -> RedisResult {
459469
.get_value::<RedisJSON>(&REDIS_JSON_TYPE)?
460470
.ok_or_else(RedisError::nonexistent_key)
461471
.and_then(|doc| {
462-
doc.value_op(&path, |value| do_json_arr_append(args.clone(), value))
463-
.map(|v| {
472+
doc.value_op(
473+
&path,
474+
|value| do_json_arr_append(args.clone(), value),
475+
|result| {
464476
ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_arrappend", key.as_str());
465477
ctx.replicate_verbatim();
466-
v.as_array().map_or(usize::MAX, |v| v.len()).into()
467-
})
468-
.map_err(|e| e.into())
478+
Ok(result
479+
.as_array()
480+
.map_or(usize::MAX, |result| result.len())
481+
.into())
482+
},
483+
)
484+
.map_err(|e| e.into())
469485
})
470486
}
471487

472-
fn do_json_arr_append<I>(args: I, value: &Value) -> Result<Value, Error>
488+
fn do_json_arr_append<I>(args: I, value: &mut Value) -> Result<Value, Error>
473489
where
474490
I: Iterator<Item = String>,
475491
{
476-
value
477-
.as_array()
478-
.ok_or_else(|| err_json(value, "array"))
479-
.and_then(|curr| {
480-
let items: Vec<Value> = args
481-
.map(|json| serde_json::from_str(&json))
482-
.collect::<Result<_, _>>()?;
492+
if !value.is_array() {
493+
return Err(err_json(value, "array"));
494+
}
483495

484-
let new_value = [curr.as_slice(), &items].concat();
485-
Ok(Value::Array(new_value))
486-
})
496+
let mut items: Vec<Value> = args
497+
.map(|json| serde_json::from_str(&json))
498+
.collect::<Result<_, _>>()?;
499+
500+
let mut new_value = value.take();
501+
new_value.as_array_mut().unwrap().append(&mut items);
502+
Ok(new_value)
487503
}
488504

489505
///
@@ -530,43 +546,43 @@ fn json_arr_insert(ctx: &Context, args: Vec<String>) -> RedisResult {
530546
.get_value::<RedisJSON>(&REDIS_JSON_TYPE)?
531547
.ok_or_else(RedisError::nonexistent_key)
532548
.and_then(|doc| {
533-
doc.value_op(&path, |value| {
534-
do_json_arr_insert(args.clone(), index, value)
535-
})
536-
.map(|v| {
537-
ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_arrinsert", key.as_str());
538-
ctx.replicate_verbatim();
539-
v.as_array().map_or(usize::MAX, |v| v.len()).into()
540-
})
549+
doc.value_op(
550+
&path,
551+
|value| do_json_arr_insert(args.clone(), index, value),
552+
|result| {
553+
ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_arrinsert", key.as_str());
554+
ctx.replicate_verbatim();
555+
Ok(result
556+
.as_array()
557+
.map_or(usize::MAX, |result| result.len())
558+
.into())
559+
},
560+
)
541561
.map_err(|e| e.into())
542562
})
543563
}
544564

545-
fn do_json_arr_insert<I>(args: I, index: i64, value: &Value) -> Result<Value, Error>
565+
fn do_json_arr_insert<I>(args: I, index: i64, value: &mut Value) -> Result<Value, Error>
546566
where
547567
I: Iterator<Item = String>,
548568
{
549-
value
550-
.as_array()
551-
.ok_or_else(|| err_json(value, "array"))
552-
.and_then(|curr| {
553-
let len = curr.len() as i64;
554-
555-
if !(-len..len).contains(&index) {
556-
return Err("ERR index out of bounds".into());
557-
}
558-
559-
let index = index.normalize(len);
560-
561-
let items: Vec<Value> = args
562-
.map(|json| serde_json::from_str(&json))
563-
.collect::<Result<_, _>>()?;
564-
565-
let mut new_value = curr.to_owned();
566-
new_value.splice(index..index, items.into_iter());
567-
568-
Ok(Value::Array(new_value))
569-
})
569+
if let Some(array) = value.as_array() {
570+
// Verify legal index in bounds
571+
let len = array.len() as i64;
572+
if !(-len..len).contains(&index) {
573+
return Err("ERR index out of bounds".into());
574+
}
575+
let index = index.normalize(len);
576+
let items: Vec<Value> = args
577+
.map(|json| serde_json::from_str(&json))
578+
.collect::<Result<_, _>>()?;
579+
let mut new_value = value.take();
580+
let curr = new_value.as_array_mut().unwrap();
581+
curr.splice(index..index, items.into_iter());
582+
Ok(new_value)
583+
} else {
584+
Err(err_json(value, "array"))
585+
}
570586
}
571587

572588
///
@@ -600,38 +616,40 @@ fn json_arr_pop(ctx: &Context, args: Vec<String>) -> RedisResult {
600616
.get_value::<RedisJSON>(&REDIS_JSON_TYPE)?
601617
.ok_or_else(RedisError::nonexistent_key)
602618
.and_then(|doc| {
603-
doc.value_op(&path, |value| do_json_arr_pop(index, &mut res, value))
604-
.map(|v| {
619+
doc.value_op(
620+
&path,
621+
|value| do_json_arr_pop(index, &mut res, value),
622+
|_result| {
605623
ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_arrpop", key.as_str());
606624
ctx.replicate_verbatim();
607-
v
608-
})
609-
.map_err(|e| e.into())
625+
Ok(()) // fake result doesn't use it uses `res` instead
626+
},
627+
)
628+
.map_err(|e| e.into())
610629
})?;
611630
Ok(RedisJSON::serialize(&res, Format::JSON)?.into())
612631
}
613632

614-
fn do_json_arr_pop(mut index: i64, res: &mut Value, value: &Value) -> Result<Value, Error> {
615-
value
616-
.as_array()
617-
.ok_or_else(|| err_json(value, "array"))
618-
.and_then(|curr| {
619-
let len = curr.len() as i64;
620-
621-
index = index.min(len - 1);
622-
623-
if index < 0 {
624-
index += len;
625-
}
633+
fn do_json_arr_pop(mut index: i64, res: &mut Value, value: &mut Value) -> Result<Value, Error> {
634+
if let Some(array) = value.as_array() {
635+
// Verify legel index in bounds
636+
let len = array.len() as i64;
637+
index = index.min(len - 1);
638+
if index < 0 {
639+
index += len;
640+
}
626641

627-
if index >= len || index < 0 {
628-
return Err("ERR index out of bounds".into());
629-
}
642+
if index >= len || index < 0 {
643+
return Err("ERR index out of bounds".into());
644+
}
630645

631-
let mut new_value = curr.to_owned();
632-
*res = new_value.remove(index as usize);
633-
Ok(Value::Array(new_value))
634-
})
646+
let mut new_value = value.take();
647+
let curr = new_value.as_array_mut().unwrap();
648+
*res = curr.remove(index as usize);
649+
Ok(new_value)
650+
} else {
651+
Err(err_json(value, "array"))
652+
}
635653
}
636654

637655
///
@@ -651,33 +669,42 @@ fn json_arr_trim(ctx: &Context, args: Vec<String>) -> RedisResult {
651669
.get_value::<RedisJSON>(&REDIS_JSON_TYPE)?
652670
.ok_or_else(RedisError::nonexistent_key)
653671
.and_then(|doc| {
654-
doc.value_op(&path, |value| do_json_arr_trim(start, stop, &value))
655-
.map(|v| {
672+
doc.value_op(
673+
&path,
674+
|value| do_json_arr_trim(start, stop, value),
675+
|result| {
656676
ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_arrtrim", key.as_str());
657677
ctx.replicate_verbatim();
658-
v.as_array().map_or(usize::MAX, |v| v.len()).into()
659-
})
660-
.map_err(|e| e.into())
678+
Ok(result
679+
.as_array()
680+
.map_or(usize::MAX, |result| result.len())
681+
.into())
682+
},
683+
)
684+
.map_err(|e| e.into())
661685
})
662686
}
663687

664-
fn do_json_arr_trim(start: i64, stop: i64, value: &Value) -> Result<Value, Error> {
665-
value
666-
.as_array()
667-
.ok_or_else(|| err_json(value, "array"))
668-
.and_then(|curr| {
669-
let len = curr.len() as i64;
670-
let stop = stop.normalize(len);
688+
fn do_json_arr_trim(start: i64, stop: i64, value: &mut Value) -> Result<Value, Error> {
689+
if let Some(array) = value.as_array() {
690+
let len = array.len() as i64;
691+
let stop = stop.normalize(len);
671692

672-
let range = if start > len || start > stop as i64 {
673-
0..0 // Return an empty array
674-
} else {
675-
start.normalize(len)..(stop + 1)
676-
};
693+
let range = if start > len || start > stop as i64 {
694+
0..0 // Return an empty array
695+
} else {
696+
start.normalize(len)..(stop + 1)
697+
};
677698

678-
let res = &curr[range];
679-
Ok(Value::Array(res.to_vec()))
680-
})
699+
let mut new_value = value.take();
700+
let curr = new_value.as_array_mut().unwrap();
701+
curr.rotate_left(range.start);
702+
curr.resize(range.end - range.start, Value::Null);
703+
704+
Ok(new_value)
705+
} else {
706+
Err(err_json(value, "array"))
707+
}
681708
}
682709

683710
///

0 commit comments

Comments
 (0)