Activity

1. lelit commented on Oct 2, 2022

   

   lelit

   on Oct 2, 2022

   Contributor

   More actions

   At a quick glance, I do not see a reason for that: PyRJ does very little when it emits a RawJSON value, other than stringifying the value it calls the underlying writer's RawValue.

   Accidentally I notice that maybe the value's stringification is pointless, given that in the RawJSON constructor it get already checked to be a str instance... I will try to understand if that's the case.

   Just for the sake of replicating your test, how did you measure the timings and where does that @profile decorator come from?
2. lelit commented on Oct 2, 2022

   

   lelit

   on Oct 2, 2022

   Last edited by lelit

   Contributor

   More actions

   Doh, scratch the second paragraph above: it's not stringifying anything, it just encodes the value to UTF-8, and that is obviously inevitable.
3. SmartAudioTools commented on Oct 4, 2022

   

   SmartAudioTools

   on Oct 4, 2022

   Last edited by SmartAudioTools

   Author

   More actions

   I tried to optimize the C++ code and I have so far managed to speed up by 8.7 x for RawJSON object serialisation.

   1. the WriteRawValue function was poorly optimized and the reallocs each time the capacity was exceeded occupied 3/4 of the time
      I wrote a new version by adding an "Extend" method to the GenericStringBuffer class => speed x 2
      in rapidjson/writer.h:

   bool WriteRawValue(const Ch* json, size_t length) {
   PutReserve(*os_, length);
   os_->Extend(json,length);
   }
   

   In rapidjson.stringbuffer.h line, 59 :

   +	void Extend(const char* json, size_t length) {
   +		std::memcpy(stack_.Push<Ch>(length), json,length);
   +	}
   

   I also had to add it to PyWriteStreamWrapper used to write in file.

   // size (to ajust) from which it is worth the cost to build a PyMemoryView and call file.write().
   #define WORTH_BUFFER_BYPASS 256
   

       void Extend(const char* json , size_t size) {
   		if ((size < WORTH_BUFFER_BYPASS) && (cursor+size < bufferEnd)) {
   			memcpy(buffer,json,size);
   			cursor += size;
   		} else {
   			Flush(); // problème if remaining due to multiByteChar before ? 
   			// Bypass buffer and write directly in the stream
   			PyObject* c;
   			if (isBinary) {
   				c =  PyMemoryView_FromMemory(const_cast<char*>(reinterpret_cast<const char*>(json)),size,PyBUF_READ); // avoid copy 
   			} else {
   				c = PyUnicode_FromStringAndSize(json, size);// copy...
   			}
   			if (c == NULL) {
   				// Propagate the error state, it will be catched by dumps_internal()
   			} else {
   				PyObject* res = PyObject_CallMethodObjArgs(stream, write_name, c, NULL); // copy inside ? 
   				if (res == NULL) {
   					// Likewise
   				} else {
   					Py_DECREF(res);
   				}
   				Py_DECREF(c);
   			}
   		}
       }
   

   2. conversion to Utf-8 line 2972 ​​with PyUnicode_AsUTF8AndSize, does not make a copy for python strings containing only ASCII, but if we have a single non-ascci character, performance drops. So I created a new object "RawJSONBYTES" which allows to pass bytes already encoded in UTF-8, and avoid a copy in any case. RawJSONBYTES are the same object as RawJSON but takes an "S" instead of a "U" when parsing arguments. We must be able to keep a single RawJSON object so that it accepts unicode or bytes , but I did not succeed

   3. I used MemoryBuffer instead of StringBuffer for the buf variable, in the do_encode function when ensureAscii is False . (I'm not sure this is doable with ensureAscii set to True)

   4. converting back to unicode with PyUnicode_FromString(buf.GetString()) (line 3395) was also too slow.
      I accelerated using instead the PyUnicode_FromStringAndSize(buf.GetString(),buf.GetSize())) and
      PyUnicode_FromStringAndSize(buf.GetBuffer(), buf.GetSize())) fonctions depending if I used StringBuffer or MemoryBuffers.

   5. I allow rapidjson to directly return bytes with a new dumpb function, which is almost the same as internal_dumps but calls do_encode with a return_bytes option which asks to use a DUMPS_INTERNAL_CALL macro with PyBytes_FromStringAndSize instead of PyUnicode_FromStringAndSize. I have not yet tested with an Encoder to which we would have added a return_bytes option. Maybe it would be bettre to have a return_bytes parameter option to dumps instead of a new fonction, allowing user to dynamicaly choose the return type, and for consistance with Encoder.call that will return Bytes or String depending of a return_bytes constructor option.

   6. by modifying the Expand method in Stack.h to directly allocate PyBytes in memory. I think we could further speed up x2 and reach the speed of juste one bytes copy, by avoiding data copying with PyBytes_FromStringAndSize.
      we could ook this solution : Let C++ function return bytes without extra copy pybind/pybind11#1236 (comment) or use this three fonctions :

   PyBytes_FromStringAndSize(NULL,size) // create a PyBytes object 
   PyBytes_AsStringAndSize(PyObject *obj, char **buffer, Py_ssize_t *length) // retrieve buffer
   int _PyBytes_Resize // resize bytes to buffer length
   

   7. If we still want to return a string rather than a bytes, by using a flag which allows us to know if all the strings we serialized were in ascii, we can then use _PyUnicode_FromASCII(buf.GetString(), buf.GetSize ())) which will further accelerate the return to Unicode by avoiding scanning the buf in search of characters in UTF-8

   I'll stop there for now, but I think there are still a lot of possible optimizations with deeper integration of serializejson, python-rapidjson and rapidjson that would allow serializejson to be a serious replacement for cpickle in reaching almost the same performance, or even surpassing it thanks to the ultra-fast compression of blosc.
4. SmartAudioTools commented on Oct 4, 2022

   

   SmartAudioTools

   on Oct 4, 2022

   Author

   More actions

   I time with line-profiler https://pypi.org/project/line-profiler/ .
   @Profile is a decorator to put before fonctions we want to line profile .
   If you name my exemple "test_RawJSON.py" , for profiling juste use the fallowing commande : > kernprof -v -l test_RawJSON.py
5. lelit commented on Oct 4, 2022

   

   lelit

   on Oct 4, 2022

   Contributor

   More actions

   Kudos! I will try to digest your steps, and see if there is anything I can generalize and apply to PyRJ.
6. SmartAudioTools commented on Oct 4, 2022

   

   SmartAudioTools

   on Oct 4, 2022

   Last edited by SmartAudioTools

   Author

   More actions

   Do you want me to make a pull request ? I'm not sure how to include changes to rapidjson and not just in python-rapidjson.
7. lelit commented on Oct 5, 2022

   

   lelit

   on Oct 5, 2022

   Contributor

   More actions

   Wrt to optimizing Python str dump, I tried several times to rebase the commit 697c15c, that in some way is overlapping with your effort here.

   At some point (see commit e31b836) we switched from using a patched version of rapidjson to its upstream version, and that optimization was lost (see also some of the comments in issue #42).

   Due to my limited knowledge of C++ I never completed the experiment, and thus I cannot say if it would bring some advantage compared to pristine rapidjson current "1.1" that we are using now.

   Maybe we could try harder to bring back that optimization: I imagine that would give us broader advantages, not just for RawJSON.
8. SmartAudioTools commented on Oct 5, 2022

   

   SmartAudioTools

   on Oct 5, 2022

   Last edited by SmartAudioTools

   Author

   More actions

   I juste edited my comment (#169 (comment)) with new elements . I have tryed to optimizing Python str dump too (it take 10x time of a copy for simple a string..). I will write a new issue for that I think, but I don't know when I will have the time to study deeper the question. My knowledge of C++ are limited too, maybe it would be good to have the opinion of a more experienced C++ coder who also knows the python API.
9. SmartAudioTools commented on Oct 5, 2022

   

   SmartAudioTools

   on Oct 5, 2022

   Author

   More actions

   I coded a PyBytesBuffer.h new file , inspired by MemoryBuffer but with direct PyBytes allocations . I have know a x20 speedup, for a RawJSONBYTES with dumpb ! The cost is exaclty the same as for a copy :)