llama : use smart pointers for ggml resources

[slaren]

Introduce the header ggml-cpp.h to ggml which contains ready to use smart pointer types for the ggml resources, and utilize them in llama.cpp.

The motivation is to avoid leaks and simplify the code, particularly in the model loader where exceptions are frequently used.

[danbev]

I needed to add a symbolic link to ggml-cpp.h from the spm-headers directory to get this to build using xcodebuild:

$ cd spm-headers
$ ln -s ../ggml/include/ggml-cpp.h ggml-cpp.h

[danbev]

Could ctx0 be changed into a ggml_context_ptr perhaps?

[slaren]

This one is used thousands of times and it would be impractical to change all of the uses to ctx0.get(). So we would need to keep a raw pointer for ease of use, as well as the smart pointer, and keep them synchronized. At that point, there would be little benefit from using a smart pointer.

Another reason is that this struct has init and free functions instead of using the constructor and destructor, and there are comments that explicitly ask to avoid doing initialization in the constructor. However, from what I can tell, init is called exactly once right after the constructor in every instance this struct is used, followed by single call free, so I don't think there is a good reason for this, but I don't know what was the motivation to do it this way.

[danbev]

Got it, thanks for the detailed explanation!

[ggerganov]

Another reason is that this struct has init and free functions instead of using the constructor and destructor, and there are comments that explicitly ask to avoid doing initialization in the constructor. However, from what I can tell, init is called exactly once right after the constructor in every instance this struct is used, followed by single call free, so I don't think there is a good reason for this, but I don't know what was the motivation to do it this way.

I try to avoid adding to much initializing logic in the constructors - just as a rule of thumb. In this case it is pointless, but in the future if we want to start handling error states from ggml_init or if we start doing extra things that could potentially fail it might not be a good idea to put them in the constructor.

[slaren]

Normally this would be handled by throwing an exception in the constructor. I imagine that the goal here was to avoid using exceptions, but they are already used everywhere in the llama.cpp code, and they are a fundamental part of RAII, which is a fundamental part of the C++ resource management. Without throwing an exception when the construction of an object fails, you end with an object in an inconsistent or invalid state. IMO we should start using C++ properly in llama.cpp instead of as a somewhat augmented C, there are a lot of inconsistencies in the code of llama.cpp due to this, and we need to standardize on one style at some point.

[ggerganov]

I'm OK to adopt a bit more idiomatic C++ in the codebase. The current style is I think mostly influenced by my own preference and the goal to write more C-style code. Working on ggml I started to appreciate it because the code ends up much more linear and explicit. But I have experience and can easily adapt to modern C++ (not too modern though 😄). The main goal should be the codebase to be consistent and any way to improve this is welcome.