reznikmm / protobuf

The Google Protocol Buffers implementation in Ada
MIT License
32 stars 5 forks source link

High cost in call to `League.Text_Codecs.Codec` #19

Closed mgrojo closed 1 year ago

mgrojo commented 1 year ago

https://github.com/reznikmm/protobuf/blob/f44685b76403eb144a825caf054e5c1365cb059f/source/runtime/pb_support-internal.adb#L363

I think this call is what causes libleague to have more cost than libadapbrt, or even libgnat, according to Callgrind. I tried to improve that by moving the constant to the global scope, but cannot be done because the package is preelaborable. Maybe is there other way. Any idea?

reznikmm commented 1 year ago

I'll ask Vadim what can we do.

Migrate from Matreshka to an other Unicode string library? VSS?

reznikmm commented 1 year ago

can you try this for now:

diff --git a/source/runtime/pb_support-internal.adb b/source/runtime/pb_support-internal.adb
index 565cacc..0fbdb25 100644
--- a/source/runtime/pb_support-internal.adb
+++ b/source/runtime/pb_support-internal.adb
@@ -20,6 +20,7 @@
 --  FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 --  DEALINGS IN THE SOFTWARE.

+with Ada.Containers.Indefinite_Holders;
 with Ada.Unchecked_Conversion;

 with League.Text_Codecs;
@@ -98,6 +99,11 @@ package body PB_Support.Internal is
       Value : Interfaces.Unsigned_64)
         with Inline;

+   package Text_Codec_Holders is new Ada.Containers.Indefinite_Holders
+     (League.Text_Codecs.Text_Codec, League.Text_Codecs."=");
+
+   Codec : Text_Codec_Holders.Holder;
+
    ----------
    -- Size --
    ----------
@@ -357,15 +363,20 @@ package body PB_Support.Internal is

    procedure Write
      (Self  : in out Stream;
-      Value : League.Strings.Universal_String)
-   is
-      Codec : constant League.Text_Codecs.Text_Codec :=
-        League.Text_Codecs.Codec
-          (League.Strings.To_Universal_String ("utf-8"));
-      Data  : constant League.Stream_Element_Vectors.Stream_Element_Vector :=
-        Codec.Encode (Value);
+      Value : League.Strings.Universal_String) is
    begin
-      Self.Write (Data);
+      if Codec.Is_Empty then
+         Codec.Replace_Element
+           (League.Text_Codecs.Codec
+              (League.Strings.To_Universal_String ("utf-8")));
+      end if;
+
+      declare
+         Data : constant League.Stream_Element_Vectors.Stream_Element_Vector :=
+           Codec.Constant_Reference.Encode (Value);
+      begin
+         Self.Write (Data);
+      end;
    end Write;

    -----------
mgrojo commented 1 year ago

Thanks, @reznikmm. I had to move the Indefinite_Holders to the package spec, because in the body it raises a compiler bug with my GNAT version. After that, this change makes the League library to consume 8 times less than before.

reznikmm commented 1 year ago

@mgrojo Does #20 work with your compiler?

mgrojo commented 1 year ago

Yes, that branch works, thank you. The global variable could also be in the body, it's only the instantiation what is causing the compiler bug.