On threads and global variables

[dwarring]

globals

LibXML has a fair number of global variables, including such things as error states, parser callbacks, and options.

Perl 5 does try to compensate, and improve reentrant behaviour, by save and restoring state, resetting options etc.

It just a matter of how hard you push it. Something's will cause problems in a single threaded environment, e.g. trying to work with multiple concurrent push or pull parsers.

threading

In the simple case LibXML doesn't support concurrent update of read/update +DOM threads across threads. There's a native lock method that's designed to help with this. This uses libXML mutexs to lock between threads. The nodes must be identical - not just in the same tree. The simplist case is to lock the document or document root element.

LibXML has been retrofitted to context switch between threads, saving and restoring state, under the control of a mutex lock. This means that some cases, tasks which aren't re-entrant may not cause contention when performed across threads.

Locking and update needs to be explored and tested more before being documented. I'd at least like to try and come up with a simple scenario where a re-entrant problem is solved by running across threads.

[vrurg]

Without knowing that LibXML is not ready for threading I actually started using it this way. So far it performs well and the only problem I'm experiencing so far is related to use of required to overcome circular dependencies. Since require as such is not thread-safe it is causing use of VMNull object sporadically.

There are a few approaches to the problem.

1. Re-structure the code to get rid of circularities. Not sure if this is possible, but it should be. For example, LibXML::Node is often used for the purpose of importing iterate-set or iterate-list subs. These can be moved into a dedicated module (say, into LibXML::Utils). As long as neither LibXML::Node::Set, nor LibXML::Node::List are using the subs this could be the best solution as they can be just use-d then by LibXML::Util.

Perhaps other cases can be resolved in a similar way.

2. Methods like xpath-class can be replaced with attributes and turned into lazy-initialized ones with AttrX::Mooish. They would then be initialized exactly once on the first read and then just used afterwards. This would result in minimal performance impact.
3. The alternative would be to use extra checks and locks withing the xpath-class method and alikes resulting in higher performance cost.

I could try implementing either approach, but would like to avoid these not appreciated by the author. :)

[dwarring]

Thanks, I haven't taken threading much further than porting 90threads.t and following libxml documentation on threading.

I went through and made a list of required classes. I did a couple of small refactors, so please update from master.

Class               Method            Required Class
-----------------   ---------------   ----------------------
LibXML::Document    implementation    LibXML
                    parser            LibXML::Parser
LibXML::Config      have-reader       LibXML::Reader
LibXML::Node        attributes        LibXML::Attr::Map
                    xpath-class       LibXML::XPath::Context
                    sub iterate-list  LibXML::Node::List
                    sub iterate-set   LibXML::Node::Set
LibXML::Node::Set   Hash              LibXML::HashMap::NodeSet

The implementation, parser and have-reader should only be used occasionally at set up. I don't think there's performance
issues anyway.

I only added attributes method to Node the DOM lets you call it on any type of Node and return undefined if its not an element. I'd considered deprecating it anyway.

Agree with treatment of xpath-class, iterate-set and iterate-list and with either solution for xpath-class.

The LibXML::Node::Set Hash method has a another require is somewhat experimental. I don't think it's widely used yet.

I think the DOM is naturally circular and a box-class or similar will always be needed. This already has locking which is hopefully doing the job.

Your approach seems reasonable and I'm happy for any assistance.

[vrurg]

Somewhat accidentally, the refactoring took me somewhat beyond just thread-safety. As I started modifying LibXML::Config a thought crossed my mind which I already had while working on my application: the config must be local to a parser instance and shared by all objects produced from that parser.

For example, max-errors is a global value. But what if an application must process both HTML and XML documents? Whereas for HTML I would sometimes turn the errors off altogether (I used to see some really bad web sites!), XML may require more restrictive approach.

I made some few steps towards the approach where is rw methods behave differently depending on wether invoked on the class or on an instance of the class. The former sets defaults. The instance then is initialized from the defaults but could be adjusted for the needs of a particular parser.

So far so good. The problem is to propagate the config object among document-bound elements. For this an element must either now its document object, where the root config is kept, or have the config object itself. Unfortunately, both approaches require and attribute – and it is not possible for CPointer representation, which is what LibXML::Element and LibXML::Node are.

Aside of this, I'm rather bad when it comes to nativecall because I never really used it. Therefore I'm not sure what the best solution to that could be. My intention is to extract the native parts of both classes into separate ones. Something like LibXML::Raw::Node and turn method raw into $.raw.

Up to my understanding, this should change very little from the user point of view as API would remain unchanged. But what if I'm missing something? An advise would be welcomed. :)

BTW, we can use IRC. I'm hanging out on both #raku and #raku-dev and usually available to pings.

[dwarring]

I used CPointer for performance reasons. Benchmarking showed nativecast to CPointer objects to be quicker than new to P6opaque. All DOM classes other than LibXML::Document are CPointer.

These could be converted to P6opaque, to allow extra attributes, but with slower object creation.

Another approach is to change the Config to a CStruct and maintain copies within C, which can then be retrived back from the raw objects. There's an xmlRef structure defined in src\xml6_gbl.c, which could be extended.

I happy to attempt this, if you want.

[vrurg]

I used CPointer for performance reasons. Benchmarking showed nativecast to CPointer objects to be quicker than new to P6opaque.

Makes full sense since there is much less to be done to build an object.

Another approach is to change the Config to a CStruct and maintain copies within C, which can then be retrived back from the raw objects. There's an xmlRef structure defined in src\xml6_gbl.c, which could be extended.

This would, probably, be the best. Though I don't see how it would maintain a single config per document principle. But this is rather due to my lack of in general understanding of the guts.

Let me try to express the problem as I see, which is no easy as I'm still trying to wrap my mind around all this. :)

1. We set default in the global config
2. A new parser instance gets a copy of it (implementation is irrelevant here)
3. A new document object somehow has access to parser's config. It could be done via keeping a reference to the parser too
4. Any sub-node of the document can access the same config too

All this would be necessary to let appendWellBalancedChunk method of LibXML::Element to work as expected by passing the configuration the element was created with into document fragment parser.

Sorry if it all sounds clumsy, but in a way I'm trying to express it all just to get better view of the problem myself. :)

In either way I'm going to rollback all the config-related changes and produce a draft PR.

[dwarring]

Thanks. I'll look at the practicality of storing config for documents over the next few week. The _private field, which is available on all nodes, including the document root is already being used to store the (poorly named) xmlRef struct, which is owned by the Raku bindings.

[dwarring]

I had a preconception when raising this ticket that libxml was sharing global state and setting between threads.
After reviewing thread saftey and threads.

Keeping one singleton global config that is definitely wrong.

Currently parsing is protected by a lock that prevents multiple threads from parsing concurrently. But hopefully that can be relaxed after fixing these issues.

[dwarring]

LibXML 0.6.16 has been released which adds the LibXML::_Configurable role to various contexts for parsing, reading, validation and xpaths, giving them a config attribute. For example:

my LibXML::Config $config .= new: :max-errors(10000);
my LibXML $parser .= new: :$config;

The node methods that were accessing global config should (If I've got them all) now take a :$config option. For example the DOM parseBalancedChunk method now takes a :$config option.

[dwarring]

Work-in-progress on the . Some of the global settings have been made thread safe.

Input callbacks are stored in a static array within libxml and aren't thread-safe. The Raku bindings currently allow per-parser input callbacks. Simplest interim solution is to deprecate allow global configuration only.

Edit: Working on a solution where a lock is held for a short duration while the input callbacks are setup at parser creation.

[dwarring]

Current solution for input-callbacks is to allow them to be configured globally, which is thread safe.

I've introduced another config setting parser-locking, which when set disables multi-threaded parsing. This must be set to enable multiple sets of per-parser input-callbacks to avert potential thread-safety issues.

I think it would be easier to fix threaded use of multiple input callbacks properly within the libxml library itself.

[dwarring]

LibXML 0.7.0 has been released which removes parser locks, except when localized sets of input callbacks are being used.

[dwarring]

Note that Perl's XML::LibXML module also has input-callback/threading issues. A bit worse because the input callbacks are always set up from scratch, rather than being set locally.

[vrurg]

I have an interesting observation to share. With my recent attempt to make require thread-safe I have also tried to replace all use of thread-blocking Lock with a new await-based Lock::Soft. This had a very confusing consequence on LibXML with t/00threads.t failing as soon as I removed the lock from LibXML::Types::resolve-package. Took me hours looking elsewhere until after tracing down to method TagExpansion I noticed that it's bypassing to xml6_gbl_[set|get]_tag_expansion. And that neither function is using any kind of handler/descriptor. This lead me to a guess that both functions are using thread-id as the handler internally.

If I'm correct then t/00threads.t works by pure accident. Since in Raku non-blocking awaiter allows ThreadPoolScheduler to re-assign a call stack to a different thread after await succeeds, and only an acquired Lock prevents this from happening (by switching to the blocking awaiter), the test currently works because there no non-blocking await is invoked between .tag-expansion = ($i + $j + 1) %% 2; and .tag-expansion == ($i + $j + 1) %% 2;. The problem did show up immediately when Lock::Soft was used internally by the core because NativeCall is actively using require, which, in turn, results in await in CompUnit code.

I'm currently reverting back to Lock anyway, so the test would not be affected. But the problem as such will remain there. Especially as long as I see more xml6_gbl_ prefixes in LibXML::Raw.

[dwarring]

I'll have a look at xml6_gbl_[set|get]_tag_expansion and the underlying calls to xmlKeepBlanksDefaultValue which I assumed to be handling the thread access.

Edit: xmlKeepBlanksDefaultValue is a macro that expands to (*(__xmlKeepBlanksDefaultValue())) which I understand to be a thread-safe and thread specific assessor. That's the reason it's invoked via binding C code rather than directly via NativeCall.